From ac5841d0c161bbb0745ce267c3587ec1bdb330ae Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 17:40:10 -0400 Subject: [PATCH 01/19] docs: design for dive computer gear twins A dive computer that downloaded a dive should appear as equipment on that dive. Today the registry (dive_computers) and gear (equipment rows of type computer) are two unconnected tables, so a downloaded dive shows its computer in the Details card but nothing in its Equipment section. Design: a nullable dive_computers.equipment_id bridge with onDelete setNull, a deterministic v5 twin id so a synced fleet converges on one row, find-or- create resolution that adopts hand-created gear before minting, and a v168 ladder step that backfills existing logbooks from dive_data_sources rather than the primary-only dives.computer_id scalar. Notable constraints captured: the linker must run after DiveEquipmentDefaulter or it suppresses default and geofenced sets; minting only where the computer row was genuinely inserted makes a user-deleted twin permanent without tombstone machinery; and EquipmentType.computer needs an explicit 0.0 case in gear_feature.dart, since it currently falls through to a 0.5 kg dry-mass default that would silently move every downloaded dive's buoyancy. --- ...26-08-26-dive-computer-gear-twin-design.md | 479 ++++++++++++++++++ 1 file changed, 479 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md new file mode 100644 index 0000000000..d02b41524a --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -0,0 +1,479 @@ +# Dive Computer Gear Twin: Design + +**Status:** approved 2026-08-26 +**Issue:** none yet. Related to #1020 (equipment set keyed on the downloading +computer), which this does not implement and does not block. +**Branch:** `worktree-dive-computer-gear-twin` +**Schema:** claims v168. + +## Problem + +A dive computer that downloaded a dive should appear as a piece of equipment on +that dive. + +Today it does not, because Submersion models a dive computer twice in two +tables that have never been connected: + +* `dive_computers`, the device registry: BLE address, firmware, download + fingerprint, dive counters. Rows are created by BLE/USB download, by manual + add, and (since #1288) by file import. +* `equipment` rows of `EquipmentType.computer`, the gear item: purchase date, + service intervals, curated attributes. + +A downloaded dive gets `dives.computer_id` stamped and shows its computer in the +Details card, but its Equipment section stays empty unless a default equipment +set happened to apply. The computer earns no service tracking, never appears in +gear statistics, and cannot be filtered on. + +## Findings + +Every claim below was verified on this branch at `80f07e66f2f`. + +**F1. There is no bridge in either direction, and no serial matching between +the two tables.** `DiveComputers` (`database.dart:2425`) has no `equipmentId` +column; `Equipment` (`database.dart:936`) has no `diveComputerId`. +`grep -rn "DiveComputer" lib/features/equipment/` and +`grep -rn "Equipment" lib/features/dive_computer/` both return zero hits. The +only identity matching that exists (`dive_computer_repository_impl.dart:176` +`findByHardwareIdentity`, `:1602` `findOrCreateComputer`) is registry to +registry. + +Two importers already mint `EquipmentType.computer` gear rows with no link back +to the registry: `uddf_full_import_service.dart:2228` and +`macdive_value_mapper.dart:105-108`. Those rows are exactly the pre-existing +items the resolution order in D3 must adopt rather than duplicate. + +**F2. `dive_data_sources.computer_id` is the authoritative attribution, not +`dives.computer_id`.** The scalar holds only the primary computer, and +`_backfillDiveComputerIds` (`database.dart:4488`) derives it *from* +`dive_data_sources`, which is the richer table. A multi-source dive logged on +two computers has two source rows and one scalar. Attribution rides the FK, never +the `dive_computer_serial` / `dive_computer_model` snapshot columns, which are +nullable, duplicate across devices, and arrive from file imports with no +registered computer behind them (#1064). + +**F3. `createComputer` is a chokepoint for three registration seams, but it is +not the only creation site.** `ensureComputer:280` (BLE/USB download), manual +add, and `findOrRegisterImportedComputer:1735` (file import) all funnel into +`DiveComputerRepository.createComputer` (`:216`). The `beforeOpen` self-heal +does not: `imported_computer_backfill.dart:124` writes registry rows with a raw +`INSERT OR IGNORE INTO dive_computers`, bypassing the repository entirely. Any +mint hook needs both sites. + +**F4. `DiveEquipmentDefaulter` bails when the dive already has equipment.** +`dive_equipment_defaulter.dart:47` returns false if any `dive_equipment` row +exists. Anything that attaches gear *before* the defaulter silently suppresses +the user's default and geofenced equipment sets. + +**F5. Three services already run together at four non-interactive creation +seams.** `DiveEquipmentDefaulter`, `ChecklistDiveLinker` and +`DiveAltitudeEnricher` are applied as a trio at +`dive_computer_repository_impl.dart:1204` (download, inside `importProfile`'s +new-dive branch, guarded by `isNewDive` at `:1116`), +`uddf_entity_importer.dart:1602`, `dive_import_providers.dart:396`, and +`healthkit_adapter.dart:283`. + +**F6. Consolidation carries equipment across for free.** +`dive_consolidation_service.dart:457-477` unions `dive_equipment` from the +secondary dive into the target by `equipmentId` and marks each row pending. +`DiveComputerAdapter._consolidateDive:703` runs `importSingleDiveAsNew` before +`_consolidationService.apply:712`, so the snapshot already contains whatever the +temporary dive was given. No separate handling is required on this path. + +**F7. The replaceSource path attaches nothing.** A re-download that replaces a +source on an existing dive takes `importProfile`'s `isNewDive == false` branch, +so the trio at `:1204` never runs. That computer did log that dive. + +**F8. `EquipmentType.computer` enters the buoyancy model with a 0.5 kg +fallthrough.** `gear_feature_mapper.dart:21-23` returns null only for +`weights` and `tank`, so a computer becomes a `GearFeature`. +`gear_feature.dart` has no `computer` case in `_attributePrior` (`:115-163`, +falls to `default: return null`), `_typeDefault` (`:214-222`, falls to +`_ => 0.0`) or `_typeDryMass` (`:224-228`, falls to `_ => 0.5`). Both buoyancy +consumers go through that mapper: `buoyancy_twin_assembler.dart:293-294` and +`weight_planner_providers.dart:19-20`. Auto-creating gear rows would move every +downloaded dive's rig by 0.5 kg per computer. + +**F9. The dive detail Equipment section is hidden when empty, and the computer +already appears elsewhere on the page.** `dive_detail_page.dart:482-488` +returns nothing when `dive.equipment.isEmpty`; there is no empty state. +`_buildEquipmentSection:4478` renders `dive.equipment` as plain tiles routing to +`/equipment/`. Separately, `_buildLinkedComputerRow` (called from `:3124`) +renders a Details-card row routing to `/dive-computers/`. + +**F10. `getMostUsedGear` applies no type exclusion.** +`statistics_repository.dart:2063-2110` joins `equipment` to `dive_equipment` and +ranks by dive count with no `WHERE e.type` filter, so computers will rank as +ordinary gear. + +**F11. `onUpgrade` is guaranteed on restore; sync adopt bypasses the ladder +entirely.** `DatabaseService.restore:736` copies the file into place and calls +`initialize()` at `:838`, and `_openDatabase:248-282` runs the ladder whenever +the stored `user_version` is below current; `backup_service.dart:659-677` +rejects newer-than-app backups while explicitly allowing older ones. The +recurring comment that a restored database never runs `onUpgrade` +(`database.dart:8774`, `:8804`, `:8973`) means only that a file arriving already +stamped at or above current enters no ladder block. + +Sync adopt is different: `SyncService.adoptReplacedLibrary:3104` applies cloud +base and changeset payloads into the already-open database and never replaces +the file, so rows arriving by sync bypass the ladder. With +`minimumCompatibleSchemaVersion = 160` (`database.dart:3210`), a peer still on +v164 can sync new dives to an already-migrated device. + +**F12. The house style puts full-table backfills in the ladder, not +`beforeOpen`.** Precedents: v132 `_backfillBottomTimeFromProfile` (`:8387`), +v158 `_backfillProfileSourceIds` (`:8584`, two set-based `UPDATE`s, documented +as belonging "to the ladder, not to every open"), v102 +`_relinkStrandedTankPressures`, v122 `_backfillLegacyServiceSchedules`. The +inverse rule is stated repeatedly: a backfill that could resurrect user-deleted +rows must be `onUpgrade`-only (`:4334-4339`, `:3662-3664`, `:8829-8836`). Only +three data backfills live in `beforeOpen`, all local-only, HLC-neutral, and +cheap no-ops once healed. + +If a per-dive loop is unavoidable in a migration, v132 documents why it must +yield: the executor is a synchronous main-isolate `NativeDatabase`, drift's +awaits resolve in microtasks, and an unbroken microtask chain never reaches the +vsync queue, freezing the migration spinner (`database.dart:3696-3726`, fixed +with `if (processed++ % 25 == 24) await Future.delayed(Duration.zero);`). + +**F13. v168 is the next free schema version.** Main is at v164 +(`database.dart:3183`). A loop over open PR diffs returns v165 (#1290), v166 +(#1300), v167 (#1276); v161 (#1237) and v138 (#603) are stale. Grepping main +alone would have said v165 was free and walked into a silent auto-merge. + +**F14. #1297 is a working template for find-or-create with deterministic ids.** +`findOrRegisterImportedComputer:1666` matches identity in Dart (not SQL, because +stored text may carry whitespace from an older import), orders candidates +`updated_at DESC, id` so two devices pick the same row, then derives a +deterministic id and, critically, adopts the row already holding that id before +inserting. Its comment states the trap directly: the identity match reads the +row's current text while the id derives from the file's text, so renaming a +registered computer makes the match miss while the id still collides, and the +insert throws `SqliteException(1555)`. + +The deterministic id convention is `course_requirement_repository.dart:39-43`: +a static factory returning `Uuid().v5(Namespace.url.value, +'submersion::')` with a doc comment explaining convergence. +`dive_computers` deliberately has no unique index, because a unique constraint +on a replicated table makes an inbound sync insert throw instead of merge. + +## Design + +### D1. Bridge column on the registry + +Add to `DiveComputers` (`database.dart:2425`): + +```dart +/// The equipment row representing this device as gear. Seeded once at +/// registration, then owned by the user: renaming or retiring the gear item +/// never writes back here, and renaming the computer never overwrites it. +/// setNull, not cascade: deleting the gear item leaves the device registered. +TextColumn get equipmentId => text().nullable().references( + Equipment, + #id, + onDelete: KeyAction.setNull, +)(); +``` + +The registry side owns the link because it is tiny (a handful of rows per +diver), so the column is dense rather than mostly null, and because a device +pointing at its representation reads correctly while a gear item claiming a +device does not. + +Unlike the neighbouring `bluetoothAddress`, which is explicitly device-local +and must not synchronize, `equipmentId` **does** synchronize: equipment ids are +fleet-stable, and a peer receiving a `dive_computers` row whose `equipmentId` +points at nothing would hold a dangling reference. + +The `onDelete: setNull` is load-bearing. It is the mechanism, not a +convenience: it implements half of the deletion semantics in D6. + +### D2. Deterministic twin id + +```dart +/// Deterministic gear-twin id: every device derives the same equipment row for +/// a given registered computer, so two devices registering the same computer +/// converge to one row under sync upsert instead of duplicating. A minted +/// backfill row cannot use v4 for exactly this reason. +static String gearTwinIdFor(String computerId) => const Uuid().v5( + Namespace.url.value, + 'submersion:dive-computer-gear:$computerId', +); +``` + +It derives from `dive_computers.id`, which is itself stable and synced, rather +than from model or serial text, which a user can rename. + +Placement mirrors #1297: the id helper and the candidate-matching predicate live +in a plain-Dart file importable by both the repository and the migration, so the +ladder and the runtime path cannot drift apart. + +### D3. Resolution order + +`resolveGearTwin(computer)` returns an equipment id, applying these steps in +order. The order is the design; it is where the F14 rename trap lives. + +1. `computer.equipmentId` is set and that equipment row still exists. Return it. +2. An equipment row already holds `gearTwinIdFor(computer.id)`. Adopt it. This + is the branch that prevents `SqliteException(1555)` when a user has renamed + the computer, because the derived id still collides while text matching has + stopped agreeing. +3. Exactly one `equipment` row with `is_active = 1` (the `status` column is + independent and is not consulted, so a computer marked `needsService` still + matches) has `type == 'computer'`, the same + `diverId`, and matches on serial when the computer's serial is non-null, or + on normalized brand plus model when it is null. Adopt it. This is what picks + up gear the user created by hand, and the rows F1 notes that UDDF and MacDive + imports already mint. Zero or several candidates fall through: guessing + between two identical computers is worse than creating a second row. Matching + is done in Dart with the same normalization as `matchImportedComputer`, + because stored text may carry whitespace from an older import. +4. Mint at `gearTwinIdFor(computer.id)`. + +Then stamp `equipmentId` back onto the registry row. + +Minted rows carry `diverId`, `name` from the computer's name, `type: computer`, +`brand` from `manufacturer`, `model`, and `serialNumber`. Purchase and service +fields stay null. Filling those in is the user's job, and is the point of +seeding once. + +Serial matching in step 3 is deliberately conditional: libdivecomputer leaves +the serial null for many devices (#1064), so a serial-only rule would be dead +for a large share of users. + +### D4. Two mint sites + +Per F3: + +* `DiveComputerRepository.createComputer` (`:216`), covering BLE/USB download, + manual add, and file-import registration. +* `imported_computer_backfill.dart`, alongside its raw + `INSERT OR IGNORE INTO dive_computers` at `:124`. + +**Mint only where the computer row was genuinely inserted.** In the heal, if +`INSERT OR IGNORE` no-ops because the computer already exists, the twin mint is +not reached. This single rule satisfies the F12 resurrection constraint without +any tombstone lookup: a user-deleted twin cannot reappear on the next app open, +because the only path that would recreate it runs only when the computer itself +is new. + +### D5. The linker links, it never creates + +New `DiveComputerGearLinker` in +`lib/features/equipment/data/services/`, a fourth member of the F5 trio and +shaped like `ChecklistDiveLinker`: + +```dart +Future linkComputerGearForDive({ + required String diveId, + required String? diverId, +}); +``` + +It reads the dive's computers via the existing `getComputerIdsForDive` (which +reads `dive_data_sources`, per F2, so a multi-source dive links every computer +that logged it), takes each computer's `equipmentId` **as stored**, and attaches +the non-null ones via `bulkAddEquipment`. + +It performs no resolution and no minting. That is what makes deletion permanent: +creation happens once at registration, linking happens per dive, and a cleared +`equipmentId` simply produces no link. + +Like the defaulter, it is best-effort and swallows its own failures. Equipment +linking must never abort a download that has already persisted a dive. + +Unlike the defaulter, it is **not** gated on the dive having no equipment. + +### D6. Deletion semantics + +| Action | Result | +|---|---| +| User deletes the gear twin | `dive_equipment` rows cascade away (FK on `Equipment.id`); `dive_computers.equipment_id` becomes NULL via `setNull`; no later download re-mints, because `ensureComputer:234` early-returns for an already-registered computer and never reaches `createComputer` | +| User deletes the registry computer | The gear item survives untouched. It is real gear they still own, with their service history on it | +| User renames either side | Nothing propagates. Step 2 of D3 keeps resolution correct anyway | + +### D7. Seams and ordering + +The linker runs **strictly after** `DiveEquipmentDefaulter` at every seam. Per +F4, running it first would suppress the user's default and geofenced equipment +sets entirely. This ordering gets a regression test, not a comment. + +| Seam | Behaviour | +|---|---| +| `dive_computer_repository_impl.dart:1204` (download, new dive) | link after defaulter | +| `uddf_entity_importer.dart:1602` | link after defaulter | +| `dive_import_providers.dart:396` | link after defaulter | +| `healthkit_adapter.dart:283` | no-op (no registry computer); included so the trio stays uniform | +| Consolidation | covered for free, per F6 | +| `importProfile` replaceSource branch (`isNewDive == false`) | new, per F7; idempotent through `insertOnConflictUpdate` | + +### D8. Migration v168 + +Two passes in the `if (from < 168)` block, both PRAGMA-guarded like every +neighbouring helper. + +**Pass 1** resolves every existing `dive_computers` row through D3 and stamps +`equipment_id`. Bounded by device count, a handful of rows, so no event-loop +yield is needed. + +**Pass 2** is a single set-based insert over the union of both attribution +sources (F2): + +```sql +INSERT OR IGNORE INTO dive_equipment (dive_id, equipment_id) +SELECT s.dive_id, c.equipment_id + FROM dive_data_sources s + JOIN dive_computers c ON c.id = s.computer_id + WHERE c.equipment_id IS NOT NULL +UNION +SELECT d.id, c.equipment_id + FROM dives d + JOIN dive_computers c ON c.id = d.computer_id + WHERE c.equipment_id IS NOT NULL; +``` + +Being set-based, it sidesteps the F12 spinner-freeze problem entirely rather +than needing v132's `% 25` yield, and mirrors how v158 did its two `UPDATE`s. + +Ordering within the ladder: pass 2 must follow pass 1, since it reads the column +pass 1 writes. + +The claim touches six places, all of which must move together if the number is +renumbered before merge: the `currentSchemaVersion` scalar, the +`migrationVersions` ladder entry, the `_assert*` helper docstring, the +`if (from < 168)` guard and its `reportProgress` twin, the `beforeOpen` backstop +comment, and the migration test filename with its version assertions. The ladder +is monotonic and unique but **not** contiguous by design (162 is permanently +skipped, and 165 through 167 are reserved by open PRs); the audit must not +"fix" that. + +A `beforeOpen` column assert is still required, per the F11 rule that a database +arriving already stamped at or above v168 enters no ladder block. That assert is +schema-only. It adds the column if missing; it does not backfill. + +### D9. Sync + +Both the minted twin rows and the `dive_equipment` join rows are marked pending +and replicate. + +This deliberately departs from #1064's local-only, HLC-neutral heal. Two reasons +specific to this feature: + +1. A device can receive a `dive_computers` row by sync whose `equipmentId` + points at a twin it never minted. That is a dangling reference, not a missing + convenience. The twin row must travel. +2. Per F11, sync adopt bypasses the ladder. A dive downloaded by a peer still on + v164 and synced to an already-migrated device would otherwise never be + linked on that device, because the ladder has already run and the linker only + fires at local creation seams. Replicating closes the window. + +The cost is one small composite-key record per (dive, computer) pair, once, +comparable to a single bulk gear edit the app already supports. Convergence is +safe because the twin id is deterministic (D2) and `dive_equipment` has a +composite natural key, so two devices deriving the same rows upsert to one. + +### D10. Buoyancy + +Add an explicit `computer` case to both `_typeDefault` and `_typeDryMass` in +`gear_feature.dart`, each returning `0.0`, leaving the `buoyancy_kg` and +`dry_weight_kg` attribute path live so a tech diver can still model a canister +or console as real mass. + +This also changes existing users who added a computer as gear by hand: they lose +0.5 kg of rig dry mass. That is a move toward correctness, since 0.5 kg was +never a considered value for computers but the `_ => 0.5` fallthrough, and it +belongs in the release notes. + +### D11. UI + +**No change needed on the dive detail page.** `_buildEquipmentSection` renders +whatever is in `dive.equipment`, so the twin arrives as an ordinary tile with +the `Icons.watch` avatar and a "Dive Computer" trailing label. + +Two consequences are accepted deliberately rather than worked around: + +* Per F9, the Equipment section stops being conditionally hidden on downloaded + dives, because they now always have at least one item. +* The computer appears twice on the page, as a Details-card row and as a gear + tile. Both are kept: they route to different destinations answering different + questions (`/dive-computers/` for firmware and download history, + `/equipment/` for service and purchase). Deduplicating would remove the + device page's only entry point from a dive. + +**One row added** to the dive computer detail page showing its linked gear item. +The column is local, and this is the only place that explains where the new +equipment came from. Needs one new string translated across all locales. + +Per F10, computers will now rank in "Most Used Gear", usually at the top. That +is honest rather than wrong, and is left alone; it belongs in the release notes. + +## Error handling + +The linker is best-effort and swallows its own failures, matching +`DiveEquipmentDefaulter`. A download that has already persisted a dive must +never be aborted by equipment linking. + +Resolution failures inside `createComputer` are logged and swallowed: a computer +that fails to get a twin is still a correctly registered computer, and the next +migration or a manual add can heal it. Registration itself must not fail because +gear seeding did. + +The migration follows the ladder's idempotency contract, since a crash mid-ladder +leaves `user_version` unchanged and re-runs every step from the top on a fresh +connection. Pass 1 is idempotent through D3 step 1; pass 2 through +`INSERT OR IGNORE` on the composite key. + +## Testing + +Tests first, per the project guide. + +**Resolution order** (`resolveGearTwin`) +* derived-id adoption after a rename, the `1555` case from F14 +* unambiguous identity match adopts a hand-created gear item +* two identical candidates mint instead of guessing +* null-serial computers match on brand plus model +* a computer whose twin was deleted resolves to nothing and does not re-mint + +**Migration v168** +* stranded-database fixture at v167 with computers, dives, and data sources +* twins minted, `equipment_id` stamped, join rows inserted +* a multi-source dive links **both** computers (the F2 case) +* idempotent across a re-run +* ladder audit: monotonic, unique, scalar equals max, not asserted contiguous + +**Ordering** +* a downloaded dive receives its default equipment set **and** its computer, + proving the F4 suppression does not occur + +**Deletion** +* deleting the twin nulls `equipment_id`, cascades the joins, and the next + download does not re-mint +* the `beforeOpen` heal does not re-mint a deleted twin for an existing computer + +**Sync** +* two devices converge on one twin, no duplicates + +**Buoyancy** +* a computer contributes 0.0 to the buoyancy twin and the weight planner + +## Out of scope + +* **#1020**, choosing an equipment *set* based on which computer downloaded the + dive. Related, separately valuable, not blocked by this. +* Mirroring identity fields between the two rows. Explicitly rejected: it would + overwrite a user's chosen gear name and HLC-bump the fleet on every firmware + correction. +* A reverse badge on the equipment detail page, and any "recreate deleted twin" + action. Both additive later. +* Transmitters. `EquipmentType.transmitter` exists and #1223 is separately + tracking transmitter to tank mapping; nothing here touches it. +* Filtering computers out of gear statistics. + +## Follow-ups + +* Release notes must call out three user-visible changes: new gear items + appearing for registered computers, computers ranking in "Most Used Gear", and + the 0.5 kg buoyancy shift for anyone who had added a computer as gear by hand. +* If the "Most Used Gear" ranking proves unpopular, the cheapest fix is a type + exclusion in `getMostUsedGear`, not a change to the data model. From e475c3ce58da0645a1a94800aad161a9a757e5b3 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 17:57:33 -0400 Subject: [PATCH 02/19] docs: implementation plan for dive computer gear twins Ten tasks, each with its own TDD cycle and commit: identity helpers, the v168 column and sync parent ref, the find-or-create resolver, the link-only linker plus its four seams, the replaceSource path, the backfill, the imported-computer heal, the buoyancy fix, the device-page row, and whole project verification. Records five deviations from the spec, the two that matter being a required parentRefs entry the spec did not know about (without it a peer's live computer whose gear item was deleted locally dangles its FK and aborts the whole sync at COMMIT) and a serializer change the spec called for that turns out to be a no-op, since diveComputers round-trips through Drift's toJson. --- .../2026-08-26-dive-computer-gear-twin.md | 2372 +++++++++++++++++ 1 file changed, 2372 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md diff --git a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md new file mode 100644 index 0000000000..0f27965b22 --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md @@ -0,0 +1,2372 @@ +# Dive Computer Gear Twin Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** A dive computer that downloaded a dive appears as a piece of equipment on that dive. + +**Architecture:** A nullable `dive_computers.equipment_id` bridges the device registry to a real `equipment` row of type `computer` (its "gear twin"). The twin is created exactly once, at computer registration, at a deterministic UUID v5 id so a synced fleet converges on one row. A link-only service then attaches that twin to each dive at the non-interactive creation seams. A v168 migration backfills existing logbooks. + +**Tech Stack:** Flutter, Dart, Drift ORM, SQLite, Riverpod, `uuid` package. + +**Spec:** `docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md` + +## Global Constraints + +- **Schema version is v168.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300) and v167 (#1276) are claimed by open PRs, and main is at v164. Do NOT renumber without re-running that scan. +- **`minimumCompatibleSchemaVersion` stays at 160.** The rule at `database.dart:3211` says not to raise it for a new nullable column. +- **Never use em-dashes (`—`, U+2014)** in any output: code, comments, docs, commit messages. En-dashes as prose punctuation and spaced hyphens are equally forbidden. Use commas, colons, semicolons, or two sentences. +- **No emojis** in code, comments, or documentation. +- **TDD.** Write the failing test first, watch it fail, then implement. +- **Immutability.** Never mutate objects or arrays in place. +- **Run `dart format .`** from the repo root after completing any task. +- **Never pipe `flutter test` into `grep`.** The pipeline returns grep's exit status, so a failing suite reports success. Run the bare command. +- **Do not run two `flutter test` invocations at once.** Overlapping local runs produce phantom single-file failures. +- **All 11 locales** must be translated for any new string: `ar de en es fr he hu it nl pt zh` in `lib/l10n/arb/`. +- **Working directory** is `/Users/ericgriffin/repos/submersion-app/worktree-dive-computer-gear-twin` for every command. + +--- + +## File Structure + +**Create:** +- `lib/core/database/dive_computer_gear_identity.dart` (~85 lines) - pure Dart: the frozen namespace, the deterministic id, the candidate struct, the match rule. No database import, so both the repository and the migration can use it. +- `lib/core/database/dive_computer_gear_backfill.dart` (~140 lines) - the v168 two-pass backfill over a bare `DatabaseConnectionUser`. +- `lib/features/equipment/data/services/dive_computer_gear_resolver.dart` (~110 lines) - runtime find-or-create. +- `lib/features/equipment/data/services/dive_computer_gear_linker.dart` (~70 lines) - link-only, per dive. + +**Modify:** +- `lib/core/database/database.dart` - column, scalar, ladder entry, onUpgrade step, assert helper, beforeOpen backstop. +- `lib/core/database/imported_computer_backfill.dart` - mint a twin where a computer row was genuinely inserted. +- `lib/core/services/sync/sync_service.dart` - `parentRefs` entry (required, see Task 2). +- `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart` - resolver hook in `createComputer`, linker at the new-dive seam and the existing-dive tail. +- `lib/features/dive_import/data/services/uddf_entity_importer.dart`, `lib/features/dive_import/presentation/providers/dive_import_providers.dart`, `lib/features/import_wizard/data/adapters/healthkit_adapter.dart` - linker calls. +- `lib/core/buoyancy/gear_feature.dart` - `computer` cases. +- `lib/features/dive_log/presentation/pages/dive_computer_detail_page.dart` (exact path confirmed in Task 9) - linked gear row. +- `lib/l10n/arb/app_*.arb` - one new string, 11 locales. + +**Two files rather than one for the identity/backfill split** because the migration runs against a bare `DatabaseConnectionUser` with no `DatabaseService`, exactly as `imported_computer_backfill.dart` does. The shared rule lives in the identity module so the runtime path and the migration cannot drift apart. This mirrors the `imported_computer_identity.dart` / `imported_computer_backfill.dart` pair added by #1297. + +--- + +## Task 1: Gear twin identity module + +Pure Dart, no database. Two consumers (the runtime resolver in Task 3, the migration in Task 6) must apply an identical rule. + +**Files:** +- Create: `lib/core/database/dive_computer_gear_identity.dart` +- Test: `test/core/database/dive_computer_gear_identity_test.dart` + +**Interfaces:** +- Consumes: `normalizeComputerIdentityPart(String?)` from `lib/core/database/imported_computer_identity.dart`. +- Produces: + - `const String kDiveComputerGearNamespace` + - `String diveComputerGearId(String computerId)` + - `class GearTwinCandidate` with `final String id; final String? diverId; final String? brand; final String? model; final String? serialNumber;` and a `const` constructor taking `{required String id, String? diverId, String? brand, String? model, String? serialNumber}` + - `GearTwinCandidate? matchGearTwin({required String? manufacturer, required String? model, required String? serialNumber, required String? diverId, required Iterable candidates})` + +- [ ] **Step 1: Write the failing test** + +Create `test/core/database/dive_computer_gear_identity_test.dart`: + +```dart +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +GearTwinCandidate candidate( + String id, { + String? diverId, + String? brand, + String? model, + String? serialNumber, +}) => GearTwinCandidate( + id: id, + diverId: diverId, + brand: brand, + model: model, + serialNumber: serialNumber, +); + +void main() { + group('diveComputerGearId', () { + test('is stable for the same computer id', () { + expect(diveComputerGearId('comp-1'), diveComputerGearId('comp-1')); + }); + + test('differs between computers', () { + expect( + diveComputerGearId('comp-1'), + isNot(diveComputerGearId('comp-2')), + ); + }); + + test('is a v5 uuid, so every device derives the same primary key', () { + // Version nibble of a v5 uuid is the first character of group three. + expect(diveComputerGearId('comp-1').split('-')[2][0], '5'); + }); + }); + + group('matchGearTwin', () { + test('matches on serial when the computer has one', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd1', serialNumber: 'abc123'), + candidate('gear-2', diverId: 'd1', serialNumber: 'ZZZ999'), + ], + ); + expect(match?.id, 'gear-1'); + }); + + test('falls back to brand and model when the serial is null', () { + // libdivecomputer leaves the serial null for many devices (#1064), so a + // serial-only rule would be dead for a large share of users. + final match = matchGearTwin( + manufacturer: ' SHEARWATER ', + model: 'Perdix 2', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd1', brand: 'Shearwater', model: 'Perdix 2'), + ], + ); + expect(match?.id, 'gear-1'); + }); + + test('returns null when two candidates match, rather than guessing', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd1', brand: 'Shearwater', model: 'Perdix 2'), + candidate('gear-2', diverId: 'd1', brand: 'Shearwater', model: 'Perdix 2'), + ], + ); + expect(match, isNull); + }); + + test('returns null when nothing matches', () { + final match = matchGearTwin( + manufacturer: 'Suunto', + model: 'EON Core', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd1', brand: 'Shearwater', model: 'Perdix 2'), + ], + ); + expect(match, isNull); + }); + + test('never crosses diver scopes', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd2', serialNumber: 'ABC123'), + ], + ); + expect(match, isNull); + }); + + test('matches null-diver candidates to a null-diver computer', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: null, + candidates: [candidate('gear-1', serialNumber: 'ABC123')], + ); + expect(match?.id, 'gear-1'); + }); + + test('returns null when the computer has neither serial nor model', () { + final match = matchGearTwin( + manufacturer: null, + model: null, + serialNumber: null, + diverId: 'd1', + candidates: [candidate('gear-1', diverId: 'd1')], + ); + expect(match, isNull); + }); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/core/database/dive_computer_gear_identity_test.dart` +Expected: FAIL, compile error, `dive_computer_gear_identity.dart` does not exist. + +- [ ] **Step 3: Write the implementation** + +Create `lib/core/database/dive_computer_gear_identity.dart`: + +```dart +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/database/imported_computer_identity.dart'; + +/// Namespace for deterministic gear-twin ids (v168). +/// +/// Frozen: every device must derive the same equipment id for the same +/// registered computer, so changing this would fork one gear item into one per +/// device across a synced fleet. +const String kDiveComputerGearNamespace = + '9f2b6c41-7d3e-4a58-9c0f-1e5a8d47b2c6'; + +/// The id of the equipment row representing [computerId] as gear. +/// +/// Derived from the registry id, which is stable and synced, rather than from +/// model or serial text, which a user can rename. A minted row cannot use v4: +/// two devices registering the same computer would mint different primary keys +/// and duplicate instead of merging under sync upsert. +String diveComputerGearId(String computerId) => const Uuid().v5( + kDiveComputerGearNamespace, + 'submersion:dive-computer-gear:$computerId', +); + +/// An equipment row reduced to the fields the gear-twin match needs. +/// +/// Lets the rule live in one place: the repository builds these from Drift +/// rows, the v168 migration backfill from raw rows. +class GearTwinCandidate { + const GearTwinCandidate({ + required this.id, + this.diverId, + this.brand, + this.model, + this.serialNumber, + }); + + final String id; + final String? diverId; + final String? brand; + final String? model; + final String? serialNumber; +} + +/// The existing gear item that already represents this computer, if exactly +/// one does. +/// +/// Callers pass only candidates that are active equipment of type `computer`. +/// +/// The serial is the strong signal, but libdivecomputer leaves it null for many +/// devices (#1064), so a serial-only rule would be dead for a large share of +/// users. With no serial the rule falls back to brand plus model. +/// +/// Returns null when zero or several candidates match. Guessing between two +/// identical computers is worse than minting a second row: a wrong adoption +/// silently attaches one device's service history to another device's dives. +GearTwinCandidate? matchGearTwin({ + required String? manufacturer, + required String? model, + required String? serialNumber, + required String? diverId, + required Iterable candidates, +}) { + final wantDiver = normalizeComputerIdentityPart(diverId); + final wantSerial = normalizeComputerIdentityPart(serialNumber); + final wantBrand = normalizeComputerIdentityPart(manufacturer); + final wantModel = normalizeComputerIdentityPart(model); + + // With no serial and no model there is no identity to match on, and every + // blank-identity gear item would collide. + if (wantSerial.isEmpty && wantModel.isEmpty) return null; + + final matches = candidates.where((c) { + if (normalizeComputerIdentityPart(c.diverId) != wantDiver) return false; + if (wantSerial.isNotEmpty) { + return normalizeComputerIdentityPart(c.serialNumber) == wantSerial; + } + return normalizeComputerIdentityPart(c.brand) == wantBrand && + normalizeComputerIdentityPart(c.model) == wantModel; + }).toList(); + + return matches.length == 1 ? matches.first : null; +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `flutter test test/core/database/dive_computer_gear_identity_test.dart` +Expected: PASS, 9 tests. + +- [ ] **Step 5: Format and commit** + +```bash +dart format . +git add lib/core/database/dive_computer_gear_identity.dart test/core/database/dive_computer_gear_identity_test.dart +git commit -m "feat(equipment): deterministic gear-twin identity for dive computers" +``` + +--- + +## Task 2: Schema column, sync parent ref, and the v168 ladder rung + +Column only. The backfill lands in Task 6, once the resolver exists. + +**Files:** +- Modify: `lib/core/database/database.dart` +- Modify: `lib/core/services/sync/sync_service.dart` (`parentRefs`, near `:2042`) +- Test: `test/core/database/migration_v168_dive_computer_gear_test.dart` + +**Interfaces:** +- Produces: the `dive_computers.equipment_id` column; `AppDatabase.currentSchemaVersion == 168`; `_assertDiveComputerEquipmentColumn()`. + +- [ ] **Step 1: Write the failing test** + +Create `test/core/database/migration_v168_dive_computer_gear_test.dart`: + +```dart +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v168 adds `dive_computers.equipment_id`: the equipment row that represents a +/// registered computer as gear, so a downloaded dive lists the computer that +/// logged it alongside the rest of the diver's kit. Nullable with no default, +/// because a cleared value means the user deleted that gear item and it must +/// not come back. +NativeDatabase _dbAt167() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 167'); + rawDb.execute(''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute( + "INSERT INTO dive_computers (id, name, created_at, updated_at) " + "VALUES ('c1', 'My Perdix', 1, 1)", + ); + }, + ); +} + +void main() { + test('v168 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(168)); + expect(AppDatabase.migrationVersions, contains(168)); + }); + + test('a fresh database has dive_computers.equipment_id', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('equipment_id')); + }); + + test('the column is nullable and carries no default', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final column = cols.firstWhere( + (c) => c.read('name') == 'equipment_id', + ); + // A non-null default would claim every registered computer already has a + // gear item, and would resurrect one the user deleted. + expect(column.read('notnull'), 0); + expect(column.read('dflt_value'), isNull); + }); + + test('a database at v167 gains the column and keeps its rows', () async { + final db = AppDatabase(_dbAt167()); + addTearDown(db.close); + + final row = await db + .customSelect("SELECT name, equipment_id FROM dive_computers WHERE id = 'c1'") + .getSingle(); + expect(row.read('name'), 'My Perdix'); + expect(row.read('equipment_id'), isNull); + }); + + test('a database stranded at a parallel-branch v168 gains the column via ' + 'beforeOpen', () async { + // Stamped AT 168 but without the column: the onUpgrade block never runs, + // so only the beforeOpen backstop can add it. + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute(''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('equipment_id')); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Expected: FAIL, `currentSchemaVersion` is 164 and `equipment_id` is absent. + +- [ ] **Step 3: Add the column to the table class** + +In `lib/core/database/database.dart`, inside `class DiveComputers extends Table`, immediately after the `hlc` column and before the `@override Set get primaryKey` block: + +```dart + /// The equipment row representing this device as gear, its "gear twin" + /// (v168). Seeded once at registration, then owned by the user: renaming or + /// retiring the gear item never writes back here, and renaming the computer + /// never overwrites the gear name. + /// + /// Unlike [bluetoothAddress] this DOES synchronize, because equipment ids are + /// fleet-stable and a peer holding a null here would dangle the reference. + /// + /// setNull rather than cascade: deleting the gear item leaves the device + /// registered. The cleared column is also what makes that deletion permanent, + /// because only a genuine computer insert ever mints a twin. + TextColumn get equipmentId => text().nullable().references( + Equipment, + #id, + onDelete: KeyAction.setNull, + )(); +``` + +- [ ] **Step 4: Bump the scalar and add the ladder entry** + +Change `static const int currentSchemaVersion = 164;` to `= 168;`. + +Leave `minimumCompatibleSchemaVersion` at 160: the rule beside it says not to raise it for a new nullable column. + +Append to the end of the `migrationVersions` list, matching the surrounding comment style: + +```dart + // v168 (gear twins): dive_computers.equipment_id, the equipment row that + // represents a registered computer as gear, so a downloaded dive lists the + // computer that logged it alongside the rest of the diver's kit. The ladder + // is monotonic and unique but NOT contiguous: 162 is permanently skipped and + // 165 through 167 were claimed by parallel branches. Do not "fix" that. + 168, +``` + +- [ ] **Step 5: Add the assert helper** + +Add near the other `_assert*Column` helpers in `lib/core/database/database.dart`: + +```dart + /// v168: dive_computers.equipment_id (gear twins). Idempotent; safe to call + /// from both onUpgrade and the beforeOpen backstop. Nullable with no default, + /// because a null means "this computer has no gear item", which is also what + /// a user deleting the gear item leaves behind. + Future _assertDiveComputerEquipmentColumn() async { + final cols = await customSelect( + "PRAGMA table_info('dive_computers')", + ).get(); + if (cols.isEmpty) return; + final names = cols.map((c) => c.read('name')).toSet(); + if (!names.contains('equipment_id')) { + await customStatement( + 'ALTER TABLE dive_computers ADD COLUMN equipment_id TEXT', + ); + } + } +``` + +- [ ] **Step 6: Add the onUpgrade rung** + +At the end of the `onUpgrade` ladder, after the existing `if (from < 164)` block and its `reportProgress()` twin: + +```dart + if (from < 168) { + await _assertDiveComputerEquipmentColumn(); + } + if (from < 168) await reportProgress(); +``` + +- [ ] **Step 7: Add the beforeOpen backstop** + +Beside the other version backstops in `beforeOpen`: + +```dart + // v168 backstop: re-assert dive_computers.equipment_id (gear twins; + // same parallel-branch version-collision self-heal). Column only: + // backfillDiveComputerGearTwins is a full-table pass that belongs to + // the ladder, and re-running it on every open would resurrect a gear + // item the user deleted. + await _assertDiveComputerEquipmentColumn(); +``` + +- [ ] **Step 8: Register the sync parent ref** + +This is REQUIRED, not optional. `sync_parent_refs_completeness_test.dart` guards `SyncService.parentRefs` against the live schema: an FK to a deletable parent that is not registered lets a peer's live child dangle against a locally deleted parent, failing the deferred-FK COMMIT with `SqliteException(787)` and aborting the entire sync. + +In `lib/core/services/sync/sync_service.dart`, add to the `parentRefs` map: + +```dart + 'diveComputers': [ + (field: 'equipmentId', parent: 'equipment', nullable: true), + ], +``` + +No serializer change is needed: `diveComputers` round-trips through Drift's `row.toJson()` and `DiveComputer.fromJson`, so the new column flows automatically. The only hand-maintained exception is the `bluetoothAddress` strip, which does not apply here. + +- [ ] **Step 9: Run both tests to verify they pass** + +Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Expected: PASS, 5 tests. + +Run: `flutter test test/core/services/sync/sync_parent_refs_completeness_test.dart` +Expected: PASS. + +- [ ] **Step 10: Regenerate Drift code, format, and commit** + +```bash +dart run build_runner build --delete-conflicting-outputs +dart format . +git add lib/core/database/database.dart lib/core/database/database.g.dart lib/core/services/sync/sync_service.dart test/core/database/migration_v168_dive_computer_gear_test.dart +git commit -m "feat(db): add dive_computers.equipment_id gear-twin bridge at v168" +``` + +--- + +## Task 3: Gear twin resolver, wired into computer registration + +**Files:** +- Create: `lib/features/equipment/data/services/dive_computer_gear_resolver.dart` +- Modify: `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart` (`createComputer`) +- Test: `test/features/equipment/data/services/dive_computer_gear_resolver_test.dart` + +**Interfaces:** +- Consumes: `diveComputerGearId`, `GearTwinCandidate`, `matchGearTwin` from Task 1. +- Produces: `class DiveComputerGearResolver` with `Future resolveGearTwin(domain.DiveComputer computer)`. + +- [ ] **Step 1: Write the failing test** + +Create `test/features/equipment/data/services/dive_computer_gear_resolver_test.dart`: + +```dart +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart' hide DiveComputer; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_resolver.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late DiveComputerGearResolver resolver; + + setUp(() async { + db = await setUpTestDatabase(); + // Junction and equipment writes without full Diver fixtures. + await db.customStatement('PRAGMA foreign_keys = OFF'); + resolver = DiveComputerGearResolver(); + }); + tearDown(tearDownTestDatabase); + + DiveComputer computer({ + String id = 'c1', + String? diverId = 'd1', + String name = 'My Perdix', + String? manufacturer = 'Shearwater', + String? model = 'Perdix 2', + String? serialNumber, + String? equipmentId, + }) => DiveComputer( + id: id, + diverId: diverId, + name: name, + manufacturer: manufacturer, + model: model, + serialNumber: serialNumber, + equipmentId: equipmentId, + createdAt: DateTime.now(), + updatedAt: DateTime.now(), + ); + + Future insertGear( + String id, { + String? diverId = 'd1', + String type = 'computer', + String? brand, + String? model, + String? serialNumber, + bool isActive = true, + }) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db + .into(db.equipment) + .insert( + EquipmentCompanion.insert( + id: id, + diverId: Value(diverId), + name: id, + type: type, + brand: Value(brand), + model: Value(model), + serialNumber: Value(serialNumber), + isActive: Value(isActive), + createdAt: t, + updatedAt: t, + ), + ); + } + + test('mints a twin at the deterministic id when nothing matches', () async { + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + final row = await (db.select( + db.equipment, + )..where((t) => t.id.equals(id!))).getSingle(); + expect(row.type, 'computer'); + expect(row.name, 'My Perdix'); + expect(row.brand, 'Shearwater'); + expect(row.model, 'Perdix 2'); + // Seeded once, then owned by the user: service fields stay theirs to set. + expect(row.purchaseDate, isNull); + expect(row.serviceIntervalDays, isNull); + }); + + test('returns the stored link when its equipment row still exists', () async { + await insertGear('hand-made'); + + final id = await resolver.resolveGearTwin( + computer(equipmentId: 'hand-made'), + ); + + expect(id, 'hand-made'); + }); + + test('mints when the stored link points at a deleted row', () async { + final id = await resolver.resolveGearTwin(computer(equipmentId: 'gone')); + + expect(id, diveComputerGearId('c1')); + }); + + test('adopts the row holding the derived id after a rename', () async { + // The identity match reads the row's CURRENT text while the id derives + // from the computer id, so renaming makes the match miss while the id + // still collides. Without this branch the insert throws + // SqliteException(1555) UNIQUE constraint failed. + await insertGear( + diveComputerGearId('c1'), + brand: 'Totally', + model: 'Renamed', + ); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + final count = await db.customSelect('SELECT COUNT(*) AS c FROM equipment').getSingle(); + expect(count.read('c'), 1); + }); + + test('adopts an unambiguous hand-created gear item', () async { + await insertGear('hand-made', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, 'hand-made'); + }); + + test('mints rather than guessing between two identical candidates', () async { + await insertGear('one', brand: 'Shearwater', model: 'Perdix 2'); + await insertGear('two', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + }); + + test('ignores retired gear and non-computer gear when matching', () async { + await insertGear('retired', brand: 'Shearwater', model: 'Perdix 2', isActive: false); + await insertGear('a-bcd', type: 'bcd', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + }); + + test('is idempotent across repeated calls', () async { + final first = await resolver.resolveGearTwin(computer()); + final second = await resolver.resolveGearTwin(computer()); + + expect(first, second); + final count = await db.customSelect('SELECT COUNT(*) AS c FROM equipment').getSingle(); + expect(count.read('c'), 1); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/features/equipment/data/services/dive_computer_gear_resolver_test.dart` +Expected: FAIL, compile error, `dive_computer_gear_resolver.dart` does not exist and `DiveComputer` has no `equipmentId` field. + +- [ ] **Step 3: Add `equipmentId` to the DiveComputer entity** + +In `lib/features/dive_log/domain/entities/dive_computer.dart`, add the field, the constructor parameter, the `copyWith` parameter and assignment, and the `props` entry, matching how `serialNumber` is threaded through. The doc comment: + +```dart + /// The equipment row representing this device as gear (v168). Null when the + /// user has deleted that gear item, which is permanent: only a genuine + /// computer registration mints a twin. + final String? equipmentId; +``` + +Also add `equipmentId: row.equipmentId` to `_mapRowToComputer` in `dive_computer_repository_impl.dart` so reads carry it. + +- [ ] **Step 4: Write the resolver** + +Create `lib/features/equipment/data/services/dive_computer_gear_resolver.dart`: + +```dart +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart' + as domain; + +/// Resolves the equipment row that represents a registered dive computer as +/// gear, creating one if the diver does not already have a suitable item. +/// +/// Called only where a `dive_computers` row is genuinely created. That single +/// rule is what makes deleting a gear twin permanent: nothing else mints, so a +/// cleared `dive_computers.equipment_id` stays cleared. +class DiveComputerGearResolver { + DiveComputerGearResolver({SyncRepository? syncRepository}) + : _syncRepository = syncRepository ?? SyncRepository(); + + final SyncRepository _syncRepository; + final _log = LoggerService.forClass(DiveComputerGearResolver); + + AppDatabase get _db => DatabaseService.instance.database; + + /// The equipment id representing [computer], minting one when needed. + /// + /// Resolution order, which is the design: + /// 1. the stored link, when its equipment row still exists + /// 2. the row already holding the derived id, which survives a rename + /// 3. exactly one unambiguous identity match among active computer gear + /// 4. mint at the derived id + /// + /// Returns null and logs on failure. A computer that fails to get a twin is + /// still a correctly registered computer, so registration must not fail + /// because gear seeding did. + Future resolveGearTwin(domain.DiveComputer computer) async { + try { + final stored = computer.equipmentId; + if (stored != null && stored.isNotEmpty) { + final existing = await (_db.select( + _db.equipment, + )..where((t) => t.id.equals(stored))).getSingleOrNull(); + if (existing != null) return stored; + } + + final derivedId = diveComputerGearId(computer.id); + + // Step 2. The identity match below reads each row's CURRENT text while + // the id derives from the computer id, so a renamed gear item makes the + // match miss while the id still collides. Adopt the row holding it + // rather than letting the insert throw SqliteException(1555). + final byDerivedId = await (_db.select( + _db.equipment, + )..where((t) => t.id.equals(derivedId))).getSingleOrNull(); + if (byDerivedId != null) return derivedId; + + final rows = + await (_db.select(_db.equipment) + ..where((t) => t.type.equals(EquipmentType.computer.name)) + ..where((t) => t.isActive.equals(true))) + .get(); + final match = matchGearTwin( + manufacturer: computer.manufacturer, + model: computer.model, + serialNumber: computer.serialNumber, + diverId: computer.diverId, + candidates: rows.map( + (r) => GearTwinCandidate( + id: r.id, + diverId: r.diverId, + brand: r.brand, + model: r.model, + serialNumber: r.serialNumber, + ), + ), + ); + if (match != null) return match.id; + + final now = DateTime.now().millisecondsSinceEpoch; + await _db + .into(_db.equipment) + .insertOnConflictUpdate( + EquipmentCompanion.insert( + id: derivedId, + diverId: Value(computer.diverId), + name: computer.name, + type: EquipmentType.computer.name, + brand: Value(computer.manufacturer), + model: Value(computer.model), + serialNumber: Value(computer.serialNumber), + createdAt: now, + updatedAt: now, + ), + ); + await _syncRepository.markRecordPending( + entityType: 'equipment', + recordId: derivedId, + localUpdatedAt: now, + ); + return derivedId; + } catch (e, stackTrace) { + _log.error( + 'Failed to resolve a gear twin for computer ${computer.id}', + error: e, + stackTrace: stackTrace, + ); + return null; + } + } +} +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `flutter test test/features/equipment/data/services/dive_computer_gear_resolver_test.dart` +Expected: PASS, 8 tests. + +- [ ] **Step 6: Wire the resolver into `createComputer`** + +In `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart`, inside `createComputer`, immediately before the existing `_relinkOrphanedRows(...)` call: + +```dart + // Seed the gear twin once, here, because this is the only repository + // path that genuinely inserts a registry row. Minting nowhere else is + // what makes a user-deleted twin permanent. + final twinId = await DiveComputerGearResolver().resolveGearTwin(computer); + if (twinId != null) { + await _db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, computer.id], + ); + await _syncRepository.markRecordPending( + entityType: 'diveComputers', + recordId: computer.id, + localUpdatedAt: DateTime.now().millisecondsSinceEpoch, + ); + } +``` + +Add the import: + +```dart +import 'package:submersion/features/equipment/data/services/dive_computer_gear_resolver.dart'; +``` + +- [ ] **Step 7: Run the dive computer repository tests** + +Run: `flutter test test/features/dive_log/data/repositories/` +Expected: PASS. + +- [ ] **Step 8: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(equipment): seed a gear twin when a dive computer is registered" +``` + +--- + +## Task 4: The link-only linker and the four creation seams + +**Files:** +- Create: `lib/features/equipment/data/services/dive_computer_gear_linker.dart` +- Modify: `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart` (the trio at the new-dive branch) +- Modify: `lib/features/dive_import/data/services/uddf_entity_importer.dart` +- Modify: `lib/features/dive_import/presentation/providers/dive_import_providers.dart` +- Modify: `lib/features/import_wizard/data/adapters/healthkit_adapter.dart` +- Test: `test/features/equipment/data/services/dive_computer_gear_linker_test.dart` + +**Interfaces:** +- Consumes: `DiveComputerRepository.getComputerIdsForDive(String diveId)`, `DiveRepository.bulkAddEquipment(List, List)`. +- Produces: `class DiveComputerGearLinker` with `Future linkComputerGearForDive({required String diveId})`. + +Note the signature takes no `diverId`. The twin is read off the computer row, and `_updateExistingDive` does not pass a `diverId` down to `importProfile`, so requiring one would block Task 5. + +- [ ] **Step 1: Write the failing test** + +Create `test/features/equipment/data/services/dive_computer_gear_linker_test.dart`: + +```dart +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late DiveComputerGearLinker linker; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + linker = DiveComputerGearLinker(); + }); + tearDown(tearDownTestDatabase); + + Future insertGear(String id) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db.into(db.equipment).insert( + EquipmentCompanion.insert( + id: id, + name: id, + type: 'computer', + createdAt: t, + updatedAt: t, + ), + ); + } + + Future insertComputer(String id, {String? equipmentId}) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db.into(db.diveComputers).insert( + DiveComputersCompanion.insert( + id: id, + name: id, + equipmentId: Value(equipmentId), + createdAt: t, + updatedAt: t, + ), + ); + } + + Future linkSource(String diveId, String computerId) async { + await db.customStatement( + 'INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, ' + 'created_at) VALUES (?, ?, ?, 1, 1)', + ['src-$computerId', diveId, computerId], + ); + } + + Future> equipmentOn(String diveId) async { + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).get(); + return rows.map((r) => r.equipmentId).toSet(); + } + + test('attaches the gear twin of the computer that logged the dive', () async { + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'gear-1'}); + }); + + test('attaches every computer on a multi-source dive', () async { + // dives.computer_id holds only the primary; a twin-computer diver must get + // both, which is why the linker reads dive_data_sources. + await insertGear('gear-1'); + await insertGear('gear-2'); + await insertComputer('c1', equipmentId: 'gear-1'); + await insertComputer('c2', equipmentId: 'gear-2'); + await linkSource('dive1', 'c1'); + await linkSource('dive1', 'c2'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'gear-1', 'gear-2'}); + }); + + test('adds to existing equipment rather than replacing it', () async { + // Unlike the defaulter, the linker is not gated on the dive being empty. + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + await db.into(db.diveEquipment).insert( + DiveEquipmentCompanion.insert(diveId: 'dive1', equipmentId: 'a-bcd'), + ); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'a-bcd', 'gear-1'}); + }); + + test('never creates equipment for a computer whose twin was deleted', () async { + await insertComputer('c1'); + await linkSource('dive1', 'c1'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isFalse); + expect(await equipmentOn('dive1'), isEmpty); + final count = await db.customSelect('SELECT COUNT(*) AS c FROM equipment').getSingle(); + expect(count.read('c'), 0); + }); + + test('is a no-op for a dive with no registered computer', () async { + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isFalse); + expect(await equipmentOn('dive1'), isEmpty); + }); + + test('is idempotent', () async { + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + + await linker.linkComputerGearForDive(diveId: 'dive1'); + await linker.linkComputerGearForDive(diveId: 'dive1'); + + expect(await equipmentOn('dive1'), {'gear-1'}); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/features/equipment/data/services/dive_computer_gear_linker_test.dart` +Expected: FAIL, compile error, `dive_computer_gear_linker.dart` does not exist. + +- [ ] **Step 3: Write the linker** + +Create `lib/features/equipment/data/services/dive_computer_gear_linker.dart`: + +```dart +import 'package:drift/drift.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_computer_repository_impl.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; + +/// Attaches the gear twins of the dive computers that logged a dive. +/// +/// Used by the non-interactive creation seams (dive-computer download, file +/// import), alongside [DiveEquipmentDefaulter], [ChecklistDiveLinker] and +/// [DiveAltitudeEnricher]. +/// +/// Link-only: it never creates an equipment row. Creation happens once, at +/// computer registration, so a twin the user deleted (which clears +/// `dive_computers.equipment_id`) simply produces no link and stays deleted. +class DiveComputerGearLinker { + DiveComputerGearLinker({ + DiveComputerRepository? computerRepository, + DiveRepository? diveRepository, + }) : _computers = computerRepository ?? DiveComputerRepository(), + _dives = diveRepository ?? DiveRepository(); + + final DiveComputerRepository _computers; + final DiveRepository _dives; + + AppDatabase get _db => DatabaseService.instance.database; + + /// Returns true when at least one twin was attached. + /// + /// MUST run after [DiveEquipmentDefaulter] at every seam: the defaulter bails + /// when the dive already has any `dive_equipment` row, so linking first would + /// silently suppress the diver's default and geofenced equipment sets. + /// + /// Unlike the defaulter this is NOT gated on the dive being empty: the + /// computer belongs on the dive whether or not a set already applied. + /// + /// Best-effort: any failure is swallowed so equipment linking can never abort + /// a download or import that has already persisted the dive. + Future linkComputerGearForDive({required String diveId}) async { + if (DatabaseService.instance.databaseOrNull == null) return false; + try { + // Reads dive_data_sources, not dives.computer_id, which holds only the + // primary: a dive logged on two computers must list both. + final computerIds = await _computers.getComputerIdsForDive(diveId); + if (computerIds.isEmpty) return false; + + final rows = await (_db.select( + _db.diveComputers, + )..where((t) => t.id.isIn(computerIds))).get(); + final equipmentIds = rows + .map((r) => r.equipmentId) + .whereType() + .where((id) => id.isNotEmpty) + .toSet() + .toList(); + if (equipmentIds.isEmpty) return false; + + await _dives.bulkAddEquipment([diveId], equipmentIds); + SyncEventBus.notifyLocalChange(); + return true; + } catch (_) { + // Best-effort: never let gear linking fail the dive operation. + return false; + } + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `flutter test test/features/equipment/data/services/dive_computer_gear_linker_test.dart` +Expected: PASS, 6 tests. + +- [ ] **Step 5: Write the ordering regression test** + +This is the trap the design exists to avoid. Create `test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart`: + +```dart +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart' hide EquipmentSet; +import 'package:submersion/features/equipment/data/repositories/equipment_set_repository_impl.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; +import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; +import 'package:submersion/features/equipment/domain/entities/equipment_set.dart'; + +import '../../../../helpers/test_database.dart'; + +/// The defaulter bails when the dive already has any dive_equipment row, so +/// running the gear linker FIRST would silently suppress the diver's default +/// and geofenced equipment sets. A downloaded dive must receive both. +void main() { + late AppDatabase db; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + final t = DateTime.now().millisecondsSinceEpoch; + for (final id in ['a-bcd', 'gear-1']) { + await db.into(db.equipment).insert( + EquipmentCompanion.insert( + id: id, + name: id, + type: id == 'gear-1' ? 'computer' : 'bcd', + createdAt: t, + updatedAt: t, + ), + ); + } + await db.into(db.diveComputers).insert( + DiveComputersCompanion.insert( + id: 'c1', + name: 'c1', + equipmentId: const Value('gear-1'), + createdAt: t, + updatedAt: t, + ), + ); + await db.customStatement( + "INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, " + "created_at) VALUES ('s1', 'dive1', 'c1', 1, 1)", + ); + final sets = EquipmentSetRepository(); + await sets.createSet( + EquipmentSet( + id: 'def', + diverId: 'd1', + name: 'def', + equipmentIds: const ['a-bcd'], + isDefault: true, + createdAt: DateTime.now(), + updatedAt: DateTime.now(), + ), + ); + await sets.setAsDefault('def', diverId: 'd1'); + }); + tearDown(tearDownTestDatabase); + + test('defaulter first, then linker: the dive gets BOTH', () async { + await DiveEquipmentDefaulter().applyDefaultEquipmentIfEmpty( + diveId: 'dive1', + diverId: 'd1', + divePoints: const [], + ); + await DiveComputerGearLinker().linkComputerGearForDive(diveId: 'dive1'); + + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals('dive1'))).get(); + expect(rows.map((r) => r.equipmentId).toSet(), {'a-bcd', 'gear-1'}); + }); + + test('linker first would suppress the default set, proving the order', () async { + await DiveComputerGearLinker().linkComputerGearForDive(diveId: 'dive1'); + final applied = await DiveEquipmentDefaulter().applyDefaultEquipmentIfEmpty( + diveId: 'dive1', + diverId: 'd1', + divePoints: const [], + ); + + expect(applied, isFalse); + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals('dive1'))).get(); + expect(rows.map((r) => r.equipmentId).toSet(), {'gear-1'}); + }); +} +``` + +- [ ] **Step 6: Run the ordering test** + +Run: `flutter test test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart` +Expected: PASS, 2 tests. + +- [ ] **Step 7: Wire the linker into all four seams** + +At each of the four sites, add the linker call **after** the `DiveEquipmentDefaulter` call and its siblings. Add the import +`import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart';` +to each file. + +In `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart`, after the defaulter call in the new-dive branch: + +```dart + // After the defaulter, never before: the defaulter bails on a dive + // that already has equipment, so linking first would suppress the + // diver's default and geofenced sets. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: diveId); +``` + +In `lib/features/dive_import/data/services/uddf_entity_importer.dart`, `lib/features/dive_import/presentation/providers/dive_import_providers.dart`, and `lib/features/import_wizard/data/adapters/healthkit_adapter.dart`, after each `applyForImportedDive(...)` call: + +```dart + await DiveComputerGearLinker().linkComputerGearForDive(diveId: dive.id); +``` + +The HealthKit site is a deliberate no-op: an Apple Watch dive has no registry computer, so `getComputerIdsForDive` returns empty. It is included so the seam set stays uniform and a future HealthKit registry entry works without a code change. + +- [ ] **Step 8: Run the affected suites** + +Run: `flutter test test/features/dive_log/ test/features/dive_import/ test/features/equipment/` +Expected: PASS. + +- [ ] **Step 9: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(equipment): attach dive computer gear twins at the import seams" +``` + +--- + +## Task 5: Link on the replaceSource path + +A re-download that replaces a source on an existing dive takes `importProfile`'s `isNewDive == false` branch, so the trio never runs. That computer did log that dive. + +**Files:** +- Modify: `lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart` +- Test: `test/features/dive_log/data/repositories/replace_source_gear_link_test.dart` + +**Interfaces:** +- Consumes: `DiveComputerGearLinker.linkComputerGearForDive` from Task 4. + +- [ ] **Step 1: Write the failing test** + +Create `test/features/dive_log/data/repositories/replace_source_gear_link_test.dart`: + +```dart +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_computer_repository_impl.dart'; + +import '../../../../helpers/test_database.dart'; + +/// A replaceSource re-download matches an existing dive, so importProfile takes +/// the isNewDive == false branch and the creation-seam trio never runs. The +/// computer still logged the dive, so its gear twin belongs on it. +void main() { + late AppDatabase db; + late DiveComputerRepository repo; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + repo = DiveComputerRepository(); + final t = DateTime.now().millisecondsSinceEpoch; + await db.into(db.equipment).insert( + EquipmentCompanion.insert( + id: 'gear-1', + name: 'gear-1', + type: 'computer', + createdAt: t, + updatedAt: t, + ), + ); + await db.into(db.diveComputers).insert( + DiveComputersCompanion.insert( + id: 'c1', + name: 'c1', + equipmentId: const Value('gear-1'), + createdAt: t, + updatedAt: t, + ), + ); + }); + tearDown(tearDownTestDatabase); + + test('re-importing onto an existing dive links the gear twin', () async { + final start = DateTime.fromMillisecondsSinceEpoch(1700000000000); + + // First import creates the dive. + final diveId = await repo.importProfile( + computerId: 'c1', + profileStartTime: start, + points: const [], + durationSeconds: 1800, + maxDepth: 30.0, + ); + + // Remove the link so the second pass has something to prove. + await (db.delete(db.diveEquipment)..where((t) => t.diveId.equals(diveId))).go(); + await repo.clearSourceAndProfiles(diveId: diveId, computerId: 'c1'); + + // Second import matches the same dive: the isNewDive == false branch. + final again = await repo.importProfile( + computerId: 'c1', + profileStartTime: start, + points: const [], + durationSeconds: 1800, + maxDepth: 30.0, + ); + + expect(again, diveId); + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).get(); + expect(rows.map((r) => r.equipmentId).toSet(), contains('gear-1')); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/features/dive_log/data/repositories/replace_source_gear_link_test.dart` +Expected: FAIL, the `dive_equipment` set is empty. + +- [ ] **Step 3: Add the linker call to the existing-dive tail** + +In `importProfile`, inside the existing `if (!isNewDive) { ... }` block that writes the gradient factors and marks the dive pending, after the `markRecordPending` call: + +```dart + // The data source row was re-created above, so the linker can see this + // computer again. clearSourceAndProfiles deleted it on the way in. + // Idempotent through insertOnConflictUpdate. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: diveId); +``` + +The import is already present from Task 4. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `flutter test test/features/dive_log/data/repositories/replace_source_gear_link_test.dart` +Expected: PASS. + +- [ ] **Step 5: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(equipment): link the gear twin when a dive source is replaced" +``` + +--- + +## Task 6: The v168 backfill + +**Files:** +- Create: `lib/core/database/dive_computer_gear_backfill.dart` +- Modify: `lib/core/database/database.dart` (call it from the `if (from < 168)` block) +- Test: `test/core/database/dive_computer_gear_backfill_test.dart` + +**Interfaces:** +- Consumes: `diveComputerGearId`, `GearTwinCandidate`, `matchGearTwin` from Task 1. +- Produces: `Future backfillDiveComputerGearTwins(DatabaseConnectionUser db)`. + +- [ ] **Step 1: Write the failing test** + +Create `test/core/database/dive_computer_gear_backfill_test.dart`: + +```dart +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// The v168 backfill mints a gear twin per registered computer and links it to +/// every dive that computer logged. Fixture is stamped at 167 so the ladder +/// runs the real migration. +NativeDatabase _seeded() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 167'); + rawDb.execute(''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE dives ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + computer_id TEXT, + dive_date_time INTEGER NOT NULL DEFAULT 0 + ) + '''); + rawDb.execute(''' + CREATE TABLE dive_data_sources ( + id TEXT NOT NULL PRIMARY KEY, + dive_id TEXT NOT NULL, + computer_id TEXT, + is_primary INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL DEFAULT 0 + ) + '''); + rawDb.execute(''' + CREATE TABLE dive_equipment ( + dive_id TEXT NOT NULL, + equipment_id TEXT NOT NULL, + PRIMARY KEY (dive_id, equipment_id) + ) + '''); + rawDb.execute(''' + CREATE TABLE equipment ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + type TEXT NOT NULL, + brand TEXT, + model TEXT, + serial_number TEXT, + status TEXT NOT NULL DEFAULT 'active', + purchase_currency TEXT NOT NULL DEFAULT 'USD', + notes TEXT NOT NULL DEFAULT '', + is_active INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + + rawDb.execute( + "INSERT INTO dive_computers (id, diver_id, name, manufacturer, model, " + "created_at, updated_at) VALUES " + "('c1', 'd1', 'My Perdix', 'Shearwater', 'Perdix 2', 1, 1)", + ); + rawDb.execute( + "INSERT INTO dive_computers (id, diver_id, name, manufacturer, model, " + "created_at, updated_at) VALUES " + "('c2', 'd1', 'My NERD', 'Shearwater', 'NERD 2', 1, 1)", + ); + // dive1: primary c1 only. dive2: two sources, c1 primary and c2. + rawDb.execute("INSERT INTO dives (id, diver_id, computer_id) VALUES ('dive1', 'd1', 'c1')"); + rawDb.execute("INSERT INTO dives (id, diver_id, computer_id) VALUES ('dive2', 'd1', 'c1')"); + rawDb.execute("INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, created_at) VALUES ('s1', 'dive1', 'c1', 1, 1)"); + rawDb.execute("INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, created_at) VALUES ('s2', 'dive2', 'c1', 1, 1)"); + rawDb.execute("INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, created_at) VALUES ('s3', 'dive2', 'c2', 0, 1)"); + }, + ); +} + +Future> _equipmentOn(AppDatabase db, String diveId) async { + final rows = await db + .customSelect("SELECT equipment_id FROM dive_equipment WHERE dive_id = '$diveId'") + .get(); + return rows.map((r) => r.read('equipment_id')).toSet(); +} + +void main() { + test('mints a twin per computer and links its dives', () async { + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + final c1Twin = diveComputerGearId('c1'); + final c2Twin = diveComputerGearId('c2'); + + final computers = await db + .customSelect('SELECT id, equipment_id FROM dive_computers ORDER BY id') + .get(); + expect(computers[0].read('equipment_id'), c1Twin); + expect(computers[1].read('equipment_id'), c2Twin); + + expect(await _equipmentOn(db, 'dive1'), {c1Twin}); + // A multi-source dive gets BOTH computers: dives.computer_id holds only + // the primary, so the union with dive_data_sources is what catches c2. + expect(await _equipmentOn(db, 'dive2'), {c1Twin, c2Twin}); + }); + + test('minted twins are computer-type gear carrying the device identity', () async { + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + final row = await db + .customSelect( + "SELECT name, type, brand, model FROM equipment WHERE id = '${diveComputerGearId('c1')}'", + ) + .getSingle(); + expect(row.read('type'), 'computer'); + expect(row.read('name'), 'My Perdix'); + expect(row.read('brand'), 'Shearwater'); + expect(row.read('model'), 'Perdix 2'); + }); + + test('is idempotent across a second open', () async { + final native = _seeded(); + final first = AppDatabase(native); + await first.customSelect('SELECT 1').get(); + await first.close(); + + final db = AppDatabase(native); + addTearDown(db.close); + final count = await db.customSelect('SELECT COUNT(*) AS c FROM equipment').getSingle(); + expect(count.read('c'), 2); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/core/database/dive_computer_gear_backfill_test.dart` +Expected: FAIL, `equipment_id` is null and `dive_equipment` is empty. + +- [ ] **Step 3: Write the backfill** + +Create `lib/core/database/dive_computer_gear_backfill.dart`: + +```dart +import 'package:drift/drift.dart'; + +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// Seed a gear twin for every registered dive computer and link it to the +/// dives that computer logged (v168). +/// +/// Ladder-only, never a beforeOpen backstop, for two independent reasons: +/// it is a full-table pass over every dive, and re-running it on every open +/// would resurrect a gear item the user deleted. That is the same rule +/// `_backfillLegacyServiceSchedules` and `_backfillBottomTimeFromProfile` +/// follow. +/// +/// New rows land on a deterministic id ([diveComputerGearId]), so every device +/// in a synced fleet derives the same primary key and they converge under sync +/// upsert rather than duplicating. +Future backfillDiveComputerGearTwins(DatabaseConnectionUser db) async { + // PRAGMA-guarded like every other backfill helper: the ladder runs against + // minimal fixtures and against databases caught mid-upgrade. PRAGMA + // table_info returns empty for a missing table, so probing columns covers + // both "table absent" and "column absent". + Future> columnsOf(String table) async { + final rows = await db.customSelect("PRAGMA table_info('$table')").get(); + return rows.map((c) => c.read('name')).toSet(); + } + + final computerCols = await columnsOf('dive_computers'); + if (!computerCols.containsAll({ + 'id', + 'diver_id', + 'name', + 'manufacturer', + 'model', + 'serial_number', + 'equipment_id', + })) { + return; + } + final equipmentCols = await columnsOf('equipment'); + if (!equipmentCols.containsAll({ + 'id', + 'diver_id', + 'name', + 'type', + 'brand', + 'model', + 'serial_number', + 'is_active', + 'created_at', + 'updated_at', + })) { + return; + } + final junctionCols = await columnsOf('dive_equipment'); + if (!junctionCols.containsAll({'dive_id', 'equipment_id'})) return; + + // Pass 1: resolve a twin per computer. Bounded by device count, a handful of + // rows, so no event-loop yield is needed here. + final computers = await db + .customSelect( + 'SELECT id, diver_id, name, manufacturer, model, serial_number ' + 'FROM dive_computers WHERE equipment_id IS NULL ORDER BY id', + ) + .get(); + + for (final computer in computers) { + final id = computer.read('id'); + final diverId = computer.read('diver_id'); + final name = computer.read('name'); + final manufacturer = computer.read('manufacturer'); + final model = computer.read('model'); + final serial = computer.read('serial_number'); + + final derivedId = diveComputerGearId(id); + + // Adopt the row already holding the derived id before matching on text: + // the match reads each row's CURRENT text while the id derives from the + // computer id, so a renamed gear item makes the match miss while the id + // still collides. + final byDerivedId = await db + .customSelect( + 'SELECT id FROM equipment WHERE id = ?', + variables: [Variable(derivedId)], + ) + .getSingleOrNull(); + + var twinId = byDerivedId?.read('id'); + + if (twinId == null) { + final candidateRows = await db + .customSelect( + "SELECT id, diver_id, brand, model, serial_number FROM equipment " + "WHERE type = 'computer' AND is_active = 1 ORDER BY updated_at DESC, id", + ) + .get(); + twinId = matchGearTwin( + manufacturer: manufacturer, + model: model, + serialNumber: serial, + diverId: diverId, + candidates: candidateRows.map( + (r) => GearTwinCandidate( + id: r.read('id'), + diverId: r.read('diver_id'), + brand: r.read('brand'), + model: r.read('model'), + serialNumber: r.read('serial_number'), + ), + ), + )?.id; + } + + if (twinId == null) { + twinId = derivedId; + final now = DateTime.now().millisecondsSinceEpoch; + await db.customStatement( + 'INSERT OR IGNORE INTO equipment ' + '(id, diver_id, name, type, brand, model, serial_number, status, ' + "purchase_currency, notes, is_active, created_at, updated_at) " + "VALUES (?, ?, ?, 'computer', ?, ?, ?, 'active', 'USD', '', 1, ?, ?)", + [twinId, diverId, name, manufacturer, model, serial, now, now], + ); + } + + await db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, id], + ); + } + + // Pass 2: link the dives. Set-based, so it needs no per-dive loop and no + // event-loop yield: a per-dive loop during a migration runs as one unbroken + // microtask chain and freezes the progress spinner. + // + // dives.computer_id holds only the PRIMARY computer, so it is unioned with + // dive_data_sources: a dive logged on two computers must list both. + final diveCols = await columnsOf('dives'); + if (diveCols.contains('computer_id')) { + await db.customStatement(''' + INSERT OR IGNORE INTO dive_equipment (dive_id, equipment_id) + SELECT d.id, c.equipment_id + FROM dives d + JOIN dive_computers c ON c.id = d.computer_id + WHERE c.equipment_id IS NOT NULL + '''); + } + + final sourceCols = await columnsOf('dive_data_sources'); + if (sourceCols.containsAll({'dive_id', 'computer_id'})) { + await db.customStatement(''' + INSERT OR IGNORE INTO dive_equipment (dive_id, equipment_id) + SELECT s.dive_id, c.equipment_id + FROM dive_data_sources s + JOIN dive_computers c ON c.id = s.computer_id + WHERE c.equipment_id IS NOT NULL + '''); + } +} +``` + +- [ ] **Step 4: Call it from the ladder** + +In `lib/core/database/database.dart`, add the import: + +```dart +import 'package:submersion/core/database/dive_computer_gear_backfill.dart'; +``` + +and extend the `if (from < 168)` block from Task 2 so it reads: + +```dart + if (from < 168) { + await _assertDiveComputerEquipmentColumn(); + await backfillDiveComputerGearTwins(this); + } + if (from < 168) await reportProgress(); +``` + +- [ ] **Step 5: Run the test to verify it passes** + +Run: `flutter test test/core/database/dive_computer_gear_backfill_test.dart` +Expected: PASS, 3 tests. + +- [ ] **Step 6: Re-run the v168 column test to confirm no regression** + +Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Expected: PASS, 5 tests. + +- [ ] **Step 7: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(db): backfill dive computer gear twins at v168" +``` + +--- + +## Task 7: Mint a twin from the imported-computer self-heal + +`imported_computer_backfill.dart` registers computers with a raw `INSERT OR IGNORE`, bypassing `createComputer` and therefore Task 3's hook. Mint there too, but only where the row was genuinely inserted, so a user-deleted twin cannot come back on the next app open. + +**Files:** +- Modify: `lib/core/database/imported_computer_backfill.dart` +- Test: `test/core/database/imported_computer_gear_twin_test.dart` + +- [ ] **Step 1: Write the failing test** + +Create `test/core/database/imported_computer_gear_twin_test.dart`: + +```dart +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// The #1288 self-heal registers computers named by file-imported dives with a +/// raw INSERT OR IGNORE, so it needs its own gear-twin mint. It must mint ONLY +/// where the computer row was genuinely inserted, or a user who deleted the +/// gear item would get it back on the next app open. +void main() { + test('the heal mints a twin and does not re-mint a deleted one', () async { + // Build the database, seed a file-imported dive, then reopen so beforeOpen + // runs the heal against it. + final native = NativeDatabase.memory(); + final first = AppDatabase(native); + final t = DateTime.now().millisecondsSinceEpoch; + await first.customStatement( + "INSERT INTO dives (id, diver_id, dive_computer_model, dive_date_time, " + "created_at, updated_at) VALUES ('dive1', 'd1', 'Perdix 2', 1, ?, ?)", + [t, t], + ); + await first.close(); + + final second = AppDatabase(native); + final registered = await second + .customSelect('SELECT id, equipment_id FROM dive_computers') + .get(); + expect(registered, hasLength(1)); + final computerId = registered.single.read('id'); + final twinId = registered.single.read('equipment_id'); + expect(twinId, diveComputerGearId(computerId)); + + // The user deletes the gear item. setNull clears the link. + await second.customStatement('DELETE FROM equipment WHERE id = ?', [twinId]); + await second.customStatement( + 'UPDATE dive_computers SET equipment_id = NULL WHERE id = ?', + [computerId], + ); + await second.close(); + + // Reopen: the heal must NOT resurrect it, because the computer row already + // exists and INSERT OR IGNORE changes nothing. + final third = AppDatabase(native); + addTearDown(third.close); + final after = await third + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + expect(after.read('c'), 0); + }); +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/core/database/imported_computer_gear_twin_test.dart` +Expected: FAIL, `equipment_id` is null after the heal registers the computer. + +- [ ] **Step 3: Mint the twin where the insert actually inserted** + +In `lib/core/database/imported_computer_backfill.dart`, add the import: + +```dart +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +``` + +and immediately after the existing `INSERT OR IGNORE INTO dive_computers` statement, before the `candidates = await _candidates(db);` re-read: + +```dart + // Seed the gear twin, but ONLY when that insert actually inserted. If + // INSERT OR IGNORE no-opped because the computer already exists, the + // user may have deleted its gear item deliberately, and re-minting here + // would resurrect it on every app open. + final inserted = await db + .customSelect('SELECT changes() AS changed') + .getSingle(); + if (inserted.read('changed') > 0) { + final twinId = diveComputerGearId(computerId); + await db.customStatement( + 'INSERT OR IGNORE INTO equipment ' + '(id, diver_id, name, type, brand, model, serial_number, status, ' + "purchase_currency, notes, is_active, created_at, updated_at) " + "VALUES (?, ?, ?, 'computer', NULL, ?, ?, 'active', 'USD', '', 1, ?, ?)", + [ + twinId, + diverId, + trimmedModel, + trimmedModel, + (trimmedSerial?.isEmpty ?? true) ? null : trimmedSerial, + now, + now, + ], + ); + await db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, computerId], + ); + } +``` + +Guard the whole block behind an `equipment_id` column probe at the top of `backfillImportedDiveComputers`, alongside the existing guards, so an old fixture without the v168 column skips it: + +```dart + final hasGearColumn = computerCols.contains('equipment_id'); +``` + +and wrap the mint in `if (hasGearColumn) { ... }`. + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `flutter test test/core/database/imported_computer_gear_twin_test.dart` +Expected: PASS. + +- [ ] **Step 5: Re-run the existing imported-computer suite** + +Run: `flutter test test/core/database/` +Expected: PASS. + +- [ ] **Step 6: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(db): seed gear twins from the imported-computer self-heal" +``` + +--- + +## Task 8: Keep dive computers out of the buoyancy dry mass + +**Files:** +- Modify: `lib/core/buoyancy/gear_feature.dart` +- Test: `test/core/buoyancy/gear_feature_test.dart` (add cases to the existing file; create it if absent) + +- [ ] **Step 1: Write the failing test** + +Add to `test/core/buoyancy/gear_feature_test.dart`: + +```dart + group('dive computers', () { + test('contribute no dry mass', () { + // Gear twins (v168) put a computer on every downloaded dive. The + // _typeDryMass fallthrough of 0.5 kg would silently move every diver's + // rig by that much per computer. + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Perdix 2', + ); + expect(feature.dryMassKg, 0.0); + }); + + test('contribute no buoyancy prior', () { + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Perdix 2', + ); + expect(feature.priorKg, 0.0); + }); + + test('still honour an explicit user dry weight', () { + // A canister light or bulky console is real mass; the attribute path + // stays live. + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Console', + weightKg: 1.2, + ); + expect(feature.dryMassKg, 1.2); + }); + }); +``` + +Add the imports the file needs if it is new: + +```dart +import 'package:flutter_test/flutter_test.dart'; +import 'package:submersion/core/buoyancy/gear_feature.dart'; +import 'package:submersion/core/constants/enums.dart'; +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `flutter test test/core/buoyancy/gear_feature_test.dart` +Expected: FAIL on the first case, `dryMassKg` is 0.5. + +- [ ] **Step 3: Add the explicit cases** + +In `lib/core/buoyancy/gear_feature.dart`: + +```dart + static double _typeDefault(EquipmentType type) => switch (type) { + EquipmentType.wetsuit => 4.0, + EquipmentType.drysuit => 10.0, + EquipmentType.bcd => -0.5, + EquipmentType.hood => 0.3, + EquipmentType.gloves => 0.2, + EquipmentType.boots => 0.4, + // Stated rather than left to the fallthrough, which already returns 0.0: + // gear twins (v168) make this a case readers will look for. + EquipmentType.computer => 0.0, + _ => 0.0, + }; + + static double _typeDryMass(EquipmentType type) => switch (type) { + EquipmentType.wetsuit => 2.0, + EquipmentType.drysuit => 3.0, + EquipmentType.bcd => 3.5, + // A wrist computer's dry mass is negligible against the rig, and gear + // twins (v168) put one on every downloaded dive: the 0.5 kg fallthrough + // would move every diver's buoyancy by that much per computer. An explicit + // dry_weight_kg attribute still wins, so a bulky console can be modeled. + EquipmentType.computer => 0.0, + _ => 0.5, + }; +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `flutter test test/core/buoyancy/gear_feature_test.dart` +Expected: PASS. + +- [ ] **Step 5: Run the buoyancy and weight planner suites** + +Run: `flutter test test/core/buoyancy/ test/features/weight_planner/` +Expected: PASS. + +- [ ] **Step 6: Format and commit** + +```bash +dart format . +git add -A +git commit -m "fix(buoyancy): dive computers contribute no dry mass" +``` + +--- + +## Task 9: Show the linked gear item on the dive computer detail page + +The only place that explains where the new equipment came from. + +**Files:** +- Modify: `lib/features/dive_computer/presentation/pages/device_detail_page.dart` +- Modify: `lib/l10n/arb/app_en.arb` plus the 10 other locales +- Test: `test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart` + +**Interfaces:** +- Consumes: `equipmentItemProvider` (`FutureProvider.family`) from `lib/features/equipment/presentation/providers/equipment_providers.dart:167`; `DiveComputer.equipmentId` from Task 3. + +The page's info card is built by a method with no `WidgetRef` in scope, so the row is a small private `ConsumerWidget` rather than another `_buildInfoRow` call. + +- [ ] **Step 1: Add the string to the English ARB** + +In `lib/l10n/arb/app_en.arb`: + +```json + "diveComputer_detail_linkedGear": "Gear item", + "@diveComputer_detail_linkedGear": { + "description": "Label for the equipment item that represents this dive computer in the diver's gear list" + }, +``` + +- [ ] **Step 2: Translate into all 10 remaining locales** + +Add the same key to `app_ar.arb`, `app_de.arb`, `app_es.arb`, `app_fr.arb`, `app_he.arb`, `app_hu.arb`, `app_it.arb`, `app_nl.arb`, `app_pt.arb`, `app_zh.arb`. Do NOT copy the English string: translate it. Omit the `@` metadata block in non-template files, matching the existing convention in those files. + +- [ ] **Step 3: Regenerate localizations** + +```bash +flutter gen-l10n +``` + +- [ ] **Step 4: Write the failing widget test** + +Create `test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart`: + +```dart +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/features/dive_computer/presentation/pages/device_detail_page.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; +import 'package:submersion/features/dive_log/presentation/providers/dive_computer_providers.dart'; +import 'package:submersion/features/equipment/domain/entities/equipment_item.dart'; +import 'package:submersion/features/equipment/presentation/providers/equipment_providers.dart'; +import 'package:submersion/features/settings/presentation/providers/settings_providers.dart'; +import 'package:submersion/l10n/arb/app_localizations.dart'; + +import '../../../../helpers/mock_providers.dart'; + +class _MockDiveComputerNotifier + extends StateNotifier>> + implements DiveComputerNotifier { + _MockDiveComputerNotifier() : super(const AsyncValue.data([])); + + @override + dynamic noSuchMethod(Invocation invocation) => null; +} + +DiveComputer _computer({String? equipmentId}) => DiveComputer( + id: 'comp-1', + name: 'My Perdix', + manufacturer: 'Shearwater', + model: 'Perdix 2', + equipmentId: equipmentId, + createdAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), +); + +EquipmentItem _gear() => EquipmentItem( + id: 'gear-1', + name: 'Perdix 2 (wrist)', + type: EquipmentType.computer, +); + +Widget _buildTestWidget({ + required DiveComputer computer, + EquipmentItem? gear, +}) { + final router = GoRouter( + initialLocation: '/dive-computers/comp-1', + routes: [ + GoRoute( + path: '/dive-computers/:id', + builder: (context, state) => + DeviceDetailPage(computerId: state.pathParameters['id']!), + ), + GoRoute( + path: '/equipment/:id', + builder: (context, state) => + const Scaffold(body: Text('EQUIPMENT_DETAIL_PAGE')), + ), + ], + ); + + return ProviderScope( + overrides: [ + settingsProvider.overrideWith((ref) => MockSettingsNotifier()), + diveComputerNotifierProvider.overrideWith( + (ref) => _MockDiveComputerNotifier(), + ), + diveComputerByIdProvider('comp-1').overrideWith((ref) async => computer), + equipmentItemProvider('gear-1').overrideWith((ref) async => gear), + ], + child: MaterialApp.router( + routerConfig: router, + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + locale: const Locale('en'), + ), + ); +} + +void main() { + testWidgets('shows the linked gear item', (tester) async { + await tester.pumpWidget( + _buildTestWidget(computer: _computer(equipmentId: 'gear-1'), gear: _gear()), + ); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsOneWidget); + }); + + testWidgets('taps through to the equipment detail page', (tester) async { + await tester.pumpWidget( + _buildTestWidget(computer: _computer(equipmentId: 'gear-1'), gear: _gear()), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.text('Perdix 2 (wrist)')); + await tester.pumpAndSettle(); + + expect(find.text('EQUIPMENT_DETAIL_PAGE'), findsOneWidget); + }); + + testWidgets('shows no gear row when the twin was deleted', (tester) async { + // A null equipmentId is what deleting the gear item leaves behind, and it + // is permanent: nothing re-mints outside a genuine registration. + await tester.pumpWidget(_buildTestWidget(computer: _computer())); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsNothing); + }); + + testWidgets('shows no gear row when the equipment row is missing', ( + tester, + ) async { + await tester.pumpWidget( + _buildTestWidget(computer: _computer(equipmentId: 'gear-1'), gear: null), + ); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsNothing); + }); +} +``` + +- [ ] **Step 5: Run the test to verify it fails** + +Run: `flutter test test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart` +Expected: FAIL, the gear name is not rendered. + +- [ ] **Step 6: Add the row widget** + +In `lib/features/dive_computer/presentation/pages/device_detail_page.dart`, add these imports: + +```dart +import 'package:submersion/features/equipment/presentation/providers/equipment_providers.dart'; +``` + +and add the widget at the bottom of the file: + +```dart +/// The equipment row representing this device as gear, its gear twin (v168). +/// +/// Absent when the computer has no `equipmentId`, which is what deleting the +/// gear item leaves behind and is permanent by design: only a genuine +/// registration mints a twin. +class _LinkedGearRow extends ConsumerWidget { + const _LinkedGearRow({required this.equipmentId}); + + final String equipmentId; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final theme = Theme.of(context); + final item = ref.watch(equipmentItemProvider(equipmentId)).valueOrNull; + if (item == null) return const SizedBox.shrink(); + + return InkWell( + onTap: () => context.push('/equipment/$equipmentId'), + child: Padding( + padding: const EdgeInsets.symmetric(vertical: 4), + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Text( + context.l10n.diveComputer_detail_linkedGear, + style: theme.textTheme.bodyMedium?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + ), + Row( + mainAxisSize: MainAxisSize.min, + children: [ + Text(item.name, style: theme.textTheme.bodyMedium), + const SizedBox(width: 4), + Icon( + Icons.chevron_right, + size: 18, + color: theme.colorScheme.onSurfaceVariant, + ), + ], + ), + ], + ), + ), + ); + } +} +``` + +- [ ] **Step 7: Render it in the info card** + +In the info card's `Column`, immediately after the connection `_buildInfoRow(...)` call: + +```dart + if (computer.equipmentId != null) + _LinkedGearRow(equipmentId: computer.equipmentId!), +``` + +- [ ] **Step 8: Run the test to verify it passes** + +Run: `flutter test test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart` +Expected: PASS, 4 tests. + +- [ ] **Step 9: Run the existing device detail tests for regressions** + +Run: `flutter test test/features/dive_computer/presentation/pages/` +Expected: PASS. + +- [ ] **Step 10: Format and commit** + +```bash +dart format . +git add -A +git commit -m "feat(ui): show a dive computer's linked gear item on its detail page" +``` + +--- + +## Task 10: Whole-project verification + +- [ ] **Step 1: Format the whole project** + +```bash +dart format . +``` + +- [ ] **Step 2: Analyze the whole project** + +Run: `flutter analyze` +Expected: no issues. Infos are fatal in CI, so treat any output as a failure. Do not pipe this into `grep`. + +- [ ] **Step 3: Verify the generated l10n hub is current** + +```bash +flutter gen-l10n +git diff --stat lib/l10n/ +``` +Expected: no unstaged changes. CI regenerates codegen but never verifies it, so a stale hub only surfaces at runtime. + +- [ ] **Step 4: Verify Drift codegen is current** + +```bash +dart run build_runner build --delete-conflicting-outputs +git diff --stat lib/core/database/ +``` +Expected: no unstaged changes. + +- [ ] **Step 5: Run the full test suite ONCE** + +Run: `flutter test` +Expected: PASS. + +One run is sufficient before opening a PR. Do not start a second run while this one is going: overlapping local runs produce phantom single-file failures. If a single file fails, re-run that file alone before believing it. + +- [ ] **Step 6: Confirm the schema claim is still free** + +```bash +for n in $(gh pr list --state open --json number --jq '.[].number'); do gh pr diff $n | grep -E '^\+\s*static const int currentSchemaVersion'; done +``` +Expected: no other open PR claims 168. If one does, renumber, remembering the claim touches six places: the scalar, the `migrationVersions` entry, the assert helper docstring, the `if (from < N)` guard and its `reportProgress` twin, the `beforeOpen` backstop comment, and the migration test filename with its `greaterThanOrEqualTo` and `contains` assertions. + +- [ ] **Step 7: Commit any formatting or codegen drift** + +```bash +git add -A +git commit -m "chore: format and regenerate after gear twin work" +``` + +--- + +## Deviations from the spec, and why + +Recorded so a reviewer comparing the two documents does not think something was missed. + +1. **D9's serializer change is not in this plan, because there is nothing to change.** `diveComputers` round-trips through Drift's `row.toJson()` and `DiveComputer.fromJson`; there is no hand-maintained field list. The new column flows automatically. What the spec did not know it needed is the `parentRefs` entry in Task 2 Step 8, which is mandatory: without it `sync_parent_refs_completeness_test.dart` fails, and in production a peer's live computer whose gear item was deleted locally would dangle its FK and abort the whole sync at COMMIT. + +2. **D2 uses a frozen namespace constant, not `Namespace.url.value`.** The closest sibling, `imported_computer_identity.dart`, declares `kImportedDiveComputerNamespace` as a literal UUID with a "frozen, changing this forks the fleet" comment. Task 1 follows that neighbour rather than the more distant `course_requirement_repository.dart` precedent the spec cited. The convergence property is identical. + +3. **D10's `_typeDefault` case is documentation, not a behaviour change.** That switch already returns 0.0 for computers through its `_ => 0.0` fallthrough. Only `_typeDryMass` changes anything. The plan adds both but says which is which. + +4. **The spec's "ladder audit: monotonic, unique, scalar equals max" test is not in this plan.** No such test exists in the repository; the only convention is the per-migration `expect(AppDatabase.migrationVersions, contains(N))`, which Task 2 follows. Adding a ladder audit would be a genuine improvement but is scope beyond this feature, and is listed as a follow-up below rather than smuggled in. + +5. **`DiveComputerGearLinker.linkComputerGearForDive` takes no `diverId`.** The spec's sketch included one. It is unnecessary, because the twin is read off the computer row, and requiring it would block Task 5: `_updateExistingDive` does not pass a `diverId` down to `importProfile`. + +## Follow-ups + +- A ladder audit test asserting `migrationVersions` is monotonic and unique and that `currentSchemaVersion == migrationVersions.last`, explicitly NOT asserting contiguity (162 is permanently skipped, 165 through 167 were claimed by parallel branches). +- Release notes for three user-visible changes: new gear items appearing for registered computers, computers ranking in "Most Used Gear", and the 0.5 kg buoyancy shift for anyone who had already added a computer as gear by hand. From 11dc768a414212122abfdb2a6a076499be9655b0 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:03:45 -0400 Subject: [PATCH 03/19] feat(equipment): deterministic gear-twin identity for dive computers --- .../database/dive_computer_gear_identity.dart | 82 ++++++++++ .../dive_computer_gear_identity_test.dart | 147 ++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 lib/core/database/dive_computer_gear_identity.dart create mode 100644 test/core/database/dive_computer_gear_identity_test.dart diff --git a/lib/core/database/dive_computer_gear_identity.dart b/lib/core/database/dive_computer_gear_identity.dart new file mode 100644 index 0000000000..336ac3f162 --- /dev/null +++ b/lib/core/database/dive_computer_gear_identity.dart @@ -0,0 +1,82 @@ +import 'package:uuid/uuid.dart'; + +import 'package:submersion/core/database/imported_computer_identity.dart'; + +/// Namespace for deterministic gear-twin ids (v168). +/// +/// Frozen: every device must derive the same equipment id for the same +/// registered computer, so changing this would fork one gear item into one per +/// device across a synced fleet. +const String kDiveComputerGearNamespace = + '9f2b6c41-7d3e-4a58-9c0f-1e5a8d47b2c6'; + +/// The id of the equipment row representing [computerId] as gear. +/// +/// Derived from the registry id, which is stable and synced, rather than from +/// model or serial text, which a user can rename. A minted row cannot use v4: +/// two devices registering the same computer would mint different primary keys +/// and duplicate instead of merging under sync upsert. +String diveComputerGearId(String computerId) => const Uuid().v5( + kDiveComputerGearNamespace, + 'submersion:dive-computer-gear:$computerId', +); + +/// An equipment row reduced to the fields the gear-twin match needs. +/// +/// Lets the rule live in one place: the repository builds these from Drift +/// rows, the v168 migration backfill from raw rows. +class GearTwinCandidate { + const GearTwinCandidate({ + required this.id, + this.diverId, + this.brand, + this.model, + this.serialNumber, + }); + + final String id; + final String? diverId; + final String? brand; + final String? model; + final String? serialNumber; +} + +/// The existing gear item that already represents this computer, if exactly +/// one does. +/// +/// Callers pass only candidates that are active equipment of type `computer`. +/// +/// The serial is the strong signal, but libdivecomputer leaves it null for many +/// devices (#1064), so a serial-only rule would be dead for a large share of +/// users. With no serial the rule falls back to brand plus model. +/// +/// Returns null when zero or several candidates match. Guessing between two +/// identical computers is worse than minting a second row: a wrong adoption +/// silently attaches one device's service history to another device's dives. +GearTwinCandidate? matchGearTwin({ + required String? manufacturer, + required String? model, + required String? serialNumber, + required String? diverId, + required Iterable candidates, +}) { + final wantDiver = normalizeComputerIdentityPart(diverId); + final wantSerial = normalizeComputerIdentityPart(serialNumber); + final wantBrand = normalizeComputerIdentityPart(manufacturer); + final wantModel = normalizeComputerIdentityPart(model); + + // With no serial and no model there is no identity to match on, and every + // blank-identity gear item would collide. + if (wantSerial.isEmpty && wantModel.isEmpty) return null; + + final matches = candidates.where((c) { + if (normalizeComputerIdentityPart(c.diverId) != wantDiver) return false; + if (wantSerial.isNotEmpty) { + return normalizeComputerIdentityPart(c.serialNumber) == wantSerial; + } + return normalizeComputerIdentityPart(c.brand) == wantBrand && + normalizeComputerIdentityPart(c.model) == wantModel; + }).toList(); + + return matches.length == 1 ? matches.first : null; +} diff --git a/test/core/database/dive_computer_gear_identity_test.dart b/test/core/database/dive_computer_gear_identity_test.dart new file mode 100644 index 0000000000..869e5c987b --- /dev/null +++ b/test/core/database/dive_computer_gear_identity_test.dart @@ -0,0 +1,147 @@ +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +GearTwinCandidate candidate( + String id, { + String? diverId, + String? brand, + String? model, + String? serialNumber, +}) => GearTwinCandidate( + id: id, + diverId: diverId, + brand: brand, + model: model, + serialNumber: serialNumber, +); + +void main() { + group('diveComputerGearId', () { + test('is stable for the same computer id', () { + expect(diveComputerGearId('comp-1'), diveComputerGearId('comp-1')); + }); + + test('differs between computers', () { + expect(diveComputerGearId('comp-1'), isNot(diveComputerGearId('comp-2'))); + }); + + test('is a v5 uuid, so every device derives the same primary key', () { + // Version nibble of a v5 uuid is the first character of group three. + expect(diveComputerGearId('comp-1').split('-')[2][0], '5'); + }); + }); + + group('matchGearTwin', () { + test('matches on serial when the computer has one', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd1', serialNumber: 'abc123'), + candidate('gear-2', diverId: 'd1', serialNumber: 'ZZZ999'), + ], + ); + expect(match?.id, 'gear-1'); + }); + + test('falls back to brand and model when the serial is null', () { + // libdivecomputer leaves the serial null for many devices (#1064), so a + // serial-only rule would be dead for a large share of users. + final match = matchGearTwin( + manufacturer: ' SHEARWATER ', + model: 'Perdix 2', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate( + 'gear-1', + diverId: 'd1', + brand: 'Shearwater', + model: 'Perdix 2', + ), + ], + ); + expect(match?.id, 'gear-1'); + }); + + test('returns null when two candidates match, rather than guessing', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate( + 'gear-1', + diverId: 'd1', + brand: 'Shearwater', + model: 'Perdix 2', + ), + candidate( + 'gear-2', + diverId: 'd1', + brand: 'Shearwater', + model: 'Perdix 2', + ), + ], + ); + expect(match, isNull); + }); + + test('returns null when nothing matches', () { + final match = matchGearTwin( + manufacturer: 'Suunto', + model: 'EON Core', + serialNumber: null, + diverId: 'd1', + candidates: [ + candidate( + 'gear-1', + diverId: 'd1', + brand: 'Shearwater', + model: 'Perdix 2', + ), + ], + ); + expect(match, isNull); + }); + + test('never crosses diver scopes', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: 'd1', + candidates: [ + candidate('gear-1', diverId: 'd2', serialNumber: 'ABC123'), + ], + ); + expect(match, isNull); + }); + + test('matches null-diver candidates to a null-diver computer', () { + final match = matchGearTwin( + manufacturer: 'Shearwater', + model: 'Perdix 2', + serialNumber: 'ABC123', + diverId: null, + candidates: [candidate('gear-1', serialNumber: 'ABC123')], + ); + expect(match?.id, 'gear-1'); + }); + + test('returns null when the computer has neither serial nor model', () { + final match = matchGearTwin( + manufacturer: null, + model: null, + serialNumber: null, + diverId: 'd1', + candidates: [candidate('gear-1', diverId: 'd1')], + ); + expect(match, isNull); + }); + }); +} From 27fa0116cfddb7a9ca89c4892823d7eae06fbb33 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:14:21 -0400 Subject: [PATCH 04/19] feat(db): add dive_computers.equipment_id gear-twin bridge at v169 Nullable FK to equipment with onDelete setNull, so deleting the gear item leaves the device registered and the cleared column is what makes that deletion permanent: only a genuine computer insert ever mints a twin. Registers the FK in SyncService.parentRefs, which is mandatory rather than tidy. Verified by removing it: sync_parent_refs_completeness_test fails with "diveComputers.equipmentId -> equipment (nullable=true)". Without it a peer's live computer whose gear item was deleted locally dangles the FK and aborts the whole sync at COMMIT with SqliteException(787). Claims v169, not the v168 the design and plan were written against. PR #1237 was renumbered from v161 onto v168 and pushed while this branch was being written, so the claim was invisible to the open-PR scan when it ran. Two branches writing the same scalar auto-merge with no conflict marker, so the collision would have surfaced only as a database silently skipping a rung. Design doc and plan updated to match. --- .../2026-08-26-dive-computer-gear-twin.md | 92 +++++++-------- ...26-08-26-dive-computer-gear-twin-design.md | 27 +++-- lib/core/database/database.dart | 56 +++++++++- lib/core/services/sync/sync_service.dart | 6 + ...igration_v169_dive_computer_gear_test.dart | 105 ++++++++++++++++++ 5 files changed, 229 insertions(+), 57 deletions(-) create mode 100644 test/core/database/migration_v169_dive_computer_gear_test.dart diff --git a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md index 0f27965b22..d74c996432 100644 --- a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md +++ b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md @@ -4,7 +4,7 @@ **Goal:** A dive computer that downloaded a dive appears as a piece of equipment on that dive. -**Architecture:** A nullable `dive_computers.equipment_id` bridges the device registry to a real `equipment` row of type `computer` (its "gear twin"). The twin is created exactly once, at computer registration, at a deterministic UUID v5 id so a synced fleet converges on one row. A link-only service then attaches that twin to each dive at the non-interactive creation seams. A v168 migration backfills existing logbooks. +**Architecture:** A nullable `dive_computers.equipment_id` bridges the device registry to a real `equipment` row of type `computer` (its "gear twin"). The twin is created exactly once, at computer registration, at a deterministic UUID v5 id so a synced fleet converges on one row. A link-only service then attaches that twin to each dive at the non-interactive creation seams. A v169 migration backfills existing logbooks. **Tech Stack:** Flutter, Dart, Drift ORM, SQLite, Riverpod, `uuid` package. @@ -12,7 +12,7 @@ ## Global Constraints -- **Schema version is v168.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300) and v167 (#1276) are claimed by open PRs, and main is at v164. Do NOT renumber without re-running that scan. +- **Schema version is v169.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300), v167 (#1276) and v168 (#1237) are claimed by open PRs, and main is at v164. This plan originally claimed v168; #1237 renumbered onto it mid-implementation. Do NOT renumber without re-running that scan. - **`minimumCompatibleSchemaVersion` stays at 160.** The rule at `database.dart:3211` says not to raise it for a new nullable column. - **Never use em-dashes (`—`, U+2014)** in any output: code, comments, docs, commit messages. En-dashes as prose punctuation and spaced hyphens are equally forbidden. Use commas, colons, semicolons, or two sentences. - **No emojis** in code, comments, or documentation. @@ -30,7 +30,7 @@ **Create:** - `lib/core/database/dive_computer_gear_identity.dart` (~85 lines) - pure Dart: the frozen namespace, the deterministic id, the candidate struct, the match rule. No database import, so both the repository and the migration can use it. -- `lib/core/database/dive_computer_gear_backfill.dart` (~140 lines) - the v168 two-pass backfill over a bare `DatabaseConnectionUser`. +- `lib/core/database/dive_computer_gear_backfill.dart` (~140 lines) - the v169 two-pass backfill over a bare `DatabaseConnectionUser`. - `lib/features/equipment/data/services/dive_computer_gear_resolver.dart` (~110 lines) - runtime find-or-create. - `lib/features/equipment/data/services/dive_computer_gear_linker.dart` (~70 lines) - link-only, per dive. @@ -215,7 +215,7 @@ import 'package:uuid/uuid.dart'; import 'package:submersion/core/database/imported_computer_identity.dart'; -/// Namespace for deterministic gear-twin ids (v168). +/// Namespace for deterministic gear-twin ids (v169). /// /// Frozen: every device must derive the same equipment id for the same /// registered computer, so changing this would fork one gear item into one per @@ -237,7 +237,7 @@ String diveComputerGearId(String computerId) => const Uuid().v5( /// An equipment row reduced to the fields the gear-twin match needs. /// /// Lets the rule live in one place: the repository builds these from Drift -/// rows, the v168 migration backfill from raw rows. +/// rows, the v169 migration backfill from raw rows. class GearTwinCandidate { const GearTwinCandidate({ required this.id, @@ -310,21 +310,21 @@ git commit -m "feat(equipment): deterministic gear-twin identity for dive comput --- -## Task 2: Schema column, sync parent ref, and the v168 ladder rung +## Task 2: Schema column, sync parent ref, and the v169 ladder rung Column only. The backfill lands in Task 6, once the resolver exists. **Files:** - Modify: `lib/core/database/database.dart` - Modify: `lib/core/services/sync/sync_service.dart` (`parentRefs`, near `:2042`) -- Test: `test/core/database/migration_v168_dive_computer_gear_test.dart` +- Test: `test/core/database/migration_v169_dive_computer_gear_test.dart` **Interfaces:** - Produces: the `dive_computers.equipment_id` column; `AppDatabase.currentSchemaVersion == 168`; `_assertDiveComputerEquipmentColumn()`. - [ ] **Step 1: Write the failing test** -Create `test/core/database/migration_v168_dive_computer_gear_test.dart`: +Create `test/core/database/migration_v169_dive_computer_gear_test.dart`: ```dart import 'package:drift/native.dart'; @@ -332,15 +332,15 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/database/database.dart'; -/// v168 adds `dive_computers.equipment_id`: the equipment row that represents a +/// v169 adds `dive_computers.equipment_id`: the equipment row that represents a /// registered computer as gear, so a downloaded dive lists the computer that /// logged it alongside the rest of the diver's kit. Nullable with no default, /// because a cleared value means the user deleted that gear item and it must /// not come back. -NativeDatabase _dbAt167() { +NativeDatabase _dbAt168() { return NativeDatabase.memory( setup: (rawDb) { - rawDb.execute('PRAGMA user_version = 167'); + rawDb.execute('PRAGMA user_version = 168'); rawDb.execute(''' CREATE TABLE dive_computers ( id TEXT NOT NULL PRIMARY KEY, @@ -365,9 +365,9 @@ NativeDatabase _dbAt167() { } void main() { - test('v168 is in the migration ladder', () { - expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(168)); - expect(AppDatabase.migrationVersions, contains(168)); + test('v169 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(169)); + expect(AppDatabase.migrationVersions, contains(169)); }); test('a fresh database has dive_computers.equipment_id', () async { @@ -397,8 +397,8 @@ void main() { expect(column.read('dflt_value'), isNull); }); - test('a database at v167 gains the column and keeps its rows', () async { - final db = AppDatabase(_dbAt167()); + test('a database at v168 gains the column and keeps its rows', () async { + final db = AppDatabase(_dbAt168()); addTearDown(db.close); final row = await db @@ -408,7 +408,7 @@ void main() { expect(row.read('equipment_id'), isNull); }); - test('a database stranded at a parallel-branch v168 gains the column via ' + test('a database stranded at a parallel-branch v169 gains the column via ' 'beforeOpen', () async { // Stamped AT 168 but without the column: the onUpgrade block never runs, // so only the beforeOpen backstop can add it. @@ -446,7 +446,7 @@ void main() { - [ ] **Step 2: Run the test to verify it fails** -Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Run: `flutter test test/core/database/migration_v169_dive_computer_gear_test.dart` Expected: FAIL, `currentSchemaVersion` is 164 and `equipment_id` is absent. - [ ] **Step 3: Add the column to the table class** @@ -455,7 +455,7 @@ In `lib/core/database/database.dart`, inside `class DiveComputers extends Table` ```dart /// The equipment row representing this device as gear, its "gear twin" - /// (v168). Seeded once at registration, then owned by the user: renaming or + /// (v169). Seeded once at registration, then owned by the user: renaming or /// retiring the gear item never writes back here, and renaming the computer /// never overwrites the gear name. /// @@ -481,7 +481,7 @@ Leave `minimumCompatibleSchemaVersion` at 160: the rule beside it says not to ra Append to the end of the `migrationVersions` list, matching the surrounding comment style: ```dart - // v168 (gear twins): dive_computers.equipment_id, the equipment row that + // v169 (gear twins): dive_computers.equipment_id, the equipment row that // represents a registered computer as gear, so a downloaded dive lists the // computer that logged it alongside the rest of the diver's kit. The ladder // is monotonic and unique but NOT contiguous: 162 is permanently skipped and @@ -494,7 +494,7 @@ Append to the end of the `migrationVersions` list, matching the surrounding comm Add near the other `_assert*Column` helpers in `lib/core/database/database.dart`: ```dart - /// v168: dive_computers.equipment_id (gear twins). Idempotent; safe to call + /// v169: dive_computers.equipment_id (gear twins). Idempotent; safe to call /// from both onUpgrade and the beforeOpen backstop. Nullable with no default, /// because a null means "this computer has no gear item", which is also what /// a user deleting the gear item leaves behind. @@ -517,10 +517,10 @@ Add near the other `_assert*Column` helpers in `lib/core/database/database.dart` At the end of the `onUpgrade` ladder, after the existing `if (from < 164)` block and its `reportProgress()` twin: ```dart - if (from < 168) { + if (from < 169) { await _assertDiveComputerEquipmentColumn(); } - if (from < 168) await reportProgress(); + if (from < 169) await reportProgress(); ``` - [ ] **Step 7: Add the beforeOpen backstop** @@ -528,7 +528,7 @@ At the end of the `onUpgrade` ladder, after the existing `if (from < 164)` block Beside the other version backstops in `beforeOpen`: ```dart - // v168 backstop: re-assert dive_computers.equipment_id (gear twins; + // v169 backstop: re-assert dive_computers.equipment_id (gear twins; // same parallel-branch version-collision self-heal). Column only: // backfillDiveComputerGearTwins is a full-table pass that belongs to // the ladder, and re-running it on every open would resurrect a gear @@ -552,7 +552,7 @@ No serializer change is needed: `diveComputers` round-trips through Drift's `row - [ ] **Step 9: Run both tests to verify they pass** -Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Run: `flutter test test/core/database/migration_v169_dive_computer_gear_test.dart` Expected: PASS, 5 tests. Run: `flutter test test/core/services/sync/sync_parent_refs_completeness_test.dart` @@ -563,8 +563,8 @@ Expected: PASS. ```bash dart run build_runner build --delete-conflicting-outputs dart format . -git add lib/core/database/database.dart lib/core/database/database.g.dart lib/core/services/sync/sync_service.dart test/core/database/migration_v168_dive_computer_gear_test.dart -git commit -m "feat(db): add dive_computers.equipment_id gear-twin bridge at v168" +git add lib/core/database/database.dart lib/core/database/database.g.dart lib/core/services/sync/sync_service.dart test/core/database/migration_v169_dive_computer_gear_test.dart +git commit -m "feat(db): add dive_computers.equipment_id gear-twin bridge at v169" ``` --- @@ -752,7 +752,7 @@ Expected: FAIL, compile error, `dive_computer_gear_resolver.dart` does not exist In `lib/features/dive_log/domain/entities/dive_computer.dart`, add the field, the constructor parameter, the `copyWith` parameter and assignment, and the `props` entry, matching how `serialNumber` is threaded through. The doc comment: ```dart - /// The equipment row representing this device as gear (v168). Null when the + /// The equipment row representing this device as gear (v169). Null when the /// user has deleted that gear item, which is permanent: only a genuine /// computer registration mints a twin. final String? equipmentId; @@ -1421,11 +1421,11 @@ git commit -m "feat(equipment): link the gear twin when a dive source is replace --- -## Task 6: The v168 backfill +## Task 6: The v169 backfill **Files:** - Create: `lib/core/database/dive_computer_gear_backfill.dart` -- Modify: `lib/core/database/database.dart` (call it from the `if (from < 168)` block) +- Modify: `lib/core/database/database.dart` (call it from the `if (from < 169)` block) - Test: `test/core/database/dive_computer_gear_backfill_test.dart` **Interfaces:** @@ -1443,13 +1443,13 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:submersion/core/database/database.dart'; import 'package:submersion/core/database/dive_computer_gear_identity.dart'; -/// The v168 backfill mints a gear twin per registered computer and links it to -/// every dive that computer logged. Fixture is stamped at 167 so the ladder +/// The v169 backfill mints a gear twin per registered computer and links it to +/// every dive that computer logged. Fixture is stamped at 168 so the ladder /// runs the real migration. NativeDatabase _seeded() { return NativeDatabase.memory( setup: (rawDb) { - rawDb.execute('PRAGMA user_version = 167'); + rawDb.execute('PRAGMA user_version = 168'); rawDb.execute(''' CREATE TABLE dive_computers ( id TEXT NOT NULL PRIMARY KEY, @@ -1598,7 +1598,7 @@ import 'package:drift/drift.dart'; import 'package:submersion/core/database/dive_computer_gear_identity.dart'; /// Seed a gear twin for every registered dive computer and link it to the -/// dives that computer logged (v168). +/// dives that computer logged (v169). /// /// Ladder-only, never a beforeOpen backstop, for two independent reasons: /// it is a full-table pass over every dive, and re-running it on every open @@ -1761,14 +1761,14 @@ In `lib/core/database/database.dart`, add the import: import 'package:submersion/core/database/dive_computer_gear_backfill.dart'; ``` -and extend the `if (from < 168)` block from Task 2 so it reads: +and extend the `if (from < 169)` block from Task 2 so it reads: ```dart - if (from < 168) { + if (from < 169) { await _assertDiveComputerEquipmentColumn(); await backfillDiveComputerGearTwins(this); } - if (from < 168) await reportProgress(); + if (from < 169) await reportProgress(); ``` - [ ] **Step 5: Run the test to verify it passes** @@ -1776,9 +1776,9 @@ and extend the `if (from < 168)` block from Task 2 so it reads: Run: `flutter test test/core/database/dive_computer_gear_backfill_test.dart` Expected: PASS, 3 tests. -- [ ] **Step 6: Re-run the v168 column test to confirm no regression** +- [ ] **Step 6: Re-run the v169 column test to confirm no regression** -Run: `flutter test test/core/database/migration_v168_dive_computer_gear_test.dart` +Run: `flutter test test/core/database/migration_v169_dive_computer_gear_test.dart` Expected: PASS, 5 tests. - [ ] **Step 7: Format and commit** @@ -1786,7 +1786,7 @@ Expected: PASS, 5 tests. ```bash dart format . git add -A -git commit -m "feat(db): backfill dive computer gear twins at v168" +git commit -m "feat(db): backfill dive computer gear twins at v169" ``` --- @@ -1904,7 +1904,7 @@ and immediately after the existing `INSERT OR IGNORE INTO dive_computers` statem } ``` -Guard the whole block behind an `equipment_id` column probe at the top of `backfillImportedDiveComputers`, alongside the existing guards, so an old fixture without the v168 column skips it: +Guard the whole block behind an `equipment_id` column probe at the top of `backfillImportedDiveComputers`, alongside the existing guards, so an old fixture without the v169 column skips it: ```dart final hasGearColumn = computerCols.contains('equipment_id'); @@ -1945,7 +1945,7 @@ Add to `test/core/buoyancy/gear_feature_test.dart`: ```dart group('dive computers', () { test('contribute no dry mass', () { - // Gear twins (v168) put a computer on every downloaded dive. The + // Gear twins (v169) put a computer on every downloaded dive. The // _typeDryMass fallthrough of 0.5 kg would silently move every diver's // rig by that much per computer. final feature = GearFeature.fromEquipment( @@ -2005,7 +2005,7 @@ In `lib/core/buoyancy/gear_feature.dart`: EquipmentType.gloves => 0.2, EquipmentType.boots => 0.4, // Stated rather than left to the fallthrough, which already returns 0.0: - // gear twins (v168) make this a case readers will look for. + // gear twins (v169) make this a case readers will look for. EquipmentType.computer => 0.0, _ => 0.0, }; @@ -2015,7 +2015,7 @@ In `lib/core/buoyancy/gear_feature.dart`: EquipmentType.drysuit => 3.0, EquipmentType.bcd => 3.5, // A wrist computer's dry mass is negligible against the rig, and gear - // twins (v168) put one on every downloaded dive: the 0.5 kg fallthrough + // twins (v169) put one on every downloaded dive: the 0.5 kg fallthrough // would move every diver's buoyancy by that much per computer. An explicit // dry_weight_kg attribute still wins, so a bulky console can be modeled. EquipmentType.computer => 0.0, @@ -2222,7 +2222,7 @@ import 'package:submersion/features/equipment/presentation/providers/equipment_p and add the widget at the bottom of the file: ```dart -/// The equipment row representing this device as gear, its gear twin (v168). +/// The equipment row representing this device as gear, its gear twin (v169). /// /// Absent when the computer has no `equipmentId`, which is what deleting the /// gear item leaves behind and is permanent by design: only a genuine @@ -2341,7 +2341,7 @@ One run is sufficient before opening a PR. Do not start a second run while this ```bash for n in $(gh pr list --state open --json number --jq '.[].number'); do gh pr diff $n | grep -E '^\+\s*static const int currentSchemaVersion'; done ``` -Expected: no other open PR claims 168. If one does, renumber, remembering the claim touches six places: the scalar, the `migrationVersions` entry, the assert helper docstring, the `if (from < N)` guard and its `reportProgress` twin, the `beforeOpen` backstop comment, and the migration test filename with its `greaterThanOrEqualTo` and `contains` assertions. +Expected: no other open PR claims 169. If one does, renumber, remembering the claim touches six places: the scalar, the `migrationVersions` entry, the assert helper docstring, the `if (from < N)` guard and its `reportProgress` twin, the `beforeOpen` backstop comment, and the migration test filename with its `greaterThanOrEqualTo` and `contains` assertions. - [ ] **Step 7: Commit any formatting or codegen drift** diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md index d02b41524a..dca31206cd 100644 --- a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -4,7 +4,7 @@ **Issue:** none yet. Related to #1020 (equipment set keyed on the downloading computer), which this does not implement and does not block. **Branch:** `worktree-dive-computer-gear-twin` -**Schema:** claims v168. +**Schema:** claims v169. ## Problem @@ -137,11 +137,18 @@ awaits resolve in microtasks, and an unbroken microtask chain never reaches the vsync queue, freezing the migration spinner (`database.dart:3696-3726`, fixed with `if (processed++ % 25 == 24) await Future.delayed(Duration.zero);`). -**F13. v168 is the next free schema version.** Main is at v164 +**F13. v169 is the next free schema version.** Main is at v164 (`database.dart:3183`). A loop over open PR diffs returns v165 (#1290), v166 -(#1300), v167 (#1276); v161 (#1237) and v138 (#603) are stale. Grepping main +(#1300), v167 (#1276) and v168 (#1237); v138 (#603) is stale. Grepping main alone would have said v165 was free and walked into a silent auto-merge. +**Renumbered from v168 during implementation.** The first scan saw #1237 at +v161. That PR was renumbered to v168 and pushed while this design was being +written, so the claim was invisible to both the main grep and the open-PR scan +at the moment they ran. Two branches writing the same scalar auto-merge with no +conflict marker, so the collision would have surfaced only as a database +silently skipping a rung. Re-run the scan immediately before opening the PR. + **F14. #1297 is a working template for find-or-create with deterministic ids.** `findOrRegisterImportedComputer:1666` matches identity in Dart (not SQL, because stored text may carry whitespace from an older import), orders candidates @@ -308,9 +315,9 @@ sets entirely. This ordering gets a regression test, not a comment. | Consolidation | covered for free, per F6 | | `importProfile` replaceSource branch (`isNewDive == false`) | new, per F7; idempotent through `insertOnConflictUpdate` | -### D8. Migration v168 +### D8. Migration v169 -Two passes in the `if (from < 168)` block, both PRAGMA-guarded like every +Two passes in the `if (from < 169)` block, both PRAGMA-guarded like every neighbouring helper. **Pass 1** resolves every existing `dive_computers` row through D3 and stamps @@ -342,14 +349,14 @@ pass 1 writes. The claim touches six places, all of which must move together if the number is renumbered before merge: the `currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the `_assert*` helper docstring, the -`if (from < 168)` guard and its `reportProgress` twin, the `beforeOpen` backstop +`if (from < 169)` guard and its `reportProgress` twin, the `beforeOpen` backstop comment, and the migration test filename with its version assertions. The ladder is monotonic and unique but **not** contiguous by design (162 is permanently -skipped, and 165 through 167 are reserved by open PRs); the audit must not +skipped, and 165 through 168 are reserved by open PRs); the audit must not "fix" that. A `beforeOpen` column assert is still required, per the F11 rule that a database -arriving already stamped at or above v168 enters no ladder block. That assert is +arriving already stamped at or above v169 enters no ladder block. That assert is schema-only. It adds the column if missing; it does not backfill. ### D9. Sync @@ -435,8 +442,8 @@ Tests first, per the project guide. * null-serial computers match on brand plus model * a computer whose twin was deleted resolves to nothing and does not re-mint -**Migration v168** -* stranded-database fixture at v167 with computers, dives, and data sources +**Migration v169** +* stranded-database fixture at v168 with computers, dives, and data sources * twins minted, `equipment_id` stamped, join rows inserted * a multi-source dive links **both** computers (the F2 case) * idempotent across a re-run diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 99b322ede9..f145ef9d93 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -2463,6 +2463,23 @@ class DiveComputers extends Table { /// (nullable: rows written before HLC rollout fall back to updatedAt). TextColumn get hlc => text().nullable()(); + /// The equipment row representing this device as gear, its "gear twin" + /// (v169). Seeded once at registration, then owned by the user: renaming or + /// retiring the gear item never writes back here, and renaming the computer + /// never overwrites the gear name. + /// + /// Unlike [bluetoothAddress] this DOES synchronize, because equipment ids are + /// fleet-stable and a peer holding a null here would dangle the reference. + /// + /// setNull rather than cascade: deleting the gear item leaves the device + /// registered. The cleared column is also what makes that deletion permanent, + /// because only a genuine computer insert ever mints a twin. + TextColumn get equipmentId => text().nullable().references( + Equipment, + #id, + onDelete: KeyAction.setNull, + )(); + @override Set get primaryKey => {id}; } @@ -3180,7 +3197,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 = 164; + static const int currentSchemaVersion = 169; /// The oldest schema whose reader can apply this build's sync payloads /// without loss or misinterpretation (the compatibility floor). @@ -3476,6 +3493,12 @@ class AppDatabase extends _$AppDatabase { // media item in the dive when its capture time is wrong (issue #1090). // Renumbered from 162, which #731 landed past while this branch was open. 164, + // v169 (gear twins): dive_computers.equipment_id, the equipment row that + // represents a registered computer as gear, so a downloaded dive lists the + // computer that logged it alongside the rest of the diver's kit. The ladder + // is monotonic and unique but NOT contiguous: 162 is permanently skipped, + // and 165 through 168 were claimed by parallel branches. Do not "fix" that. + 169, ]; /// Idempotent DDL for the v106 connector-suggestion columns (Lightroom @@ -4228,6 +4251,23 @@ class AppDatabase extends _$AppDatabase { } } + /// v169: dive_computers.equipment_id (gear twins). Idempotent; safe to call + /// from both onUpgrade and the beforeOpen backstop. Nullable with no default, + /// because a null means "this computer has no gear item", which is also what + /// a user deleting the gear item leaves behind. + Future _assertDiveComputerEquipmentColumn() async { + final cols = await customSelect( + "PRAGMA table_info('dive_computers')", + ).get(); + if (cols.isEmpty) return; + final names = cols.map((c) => c.read('name')).toSet(); + if (!names.contains('equipment_id')) { + await customStatement( + 'ALTER TABLE dive_computers ADD COLUMN equipment_id TEXT', + ); + } + } + /// v164: media.manual_elapsed_seconds (issue #1090). Idempotent; safe to /// call from both onUpgrade and the beforeOpen backstop. Nullable with no /// default, so every pre-existing row reads back as "position from @@ -8615,6 +8655,13 @@ class AppDatabase extends _$AppDatabase { await _assertMediaManualElapsedColumn(); } if (from < 164) await reportProgress(); + // v169: dive_computers.equipment_id (gear twins). The backfill that + // seeds the twins and links existing dives is added alongside this in + // a later step; the column has to land first. + if (from < 169) { + await _assertDiveComputerEquipmentColumn(); + } + if (from < 169) await reportProgress(); }, beforeOpen: (details) async { // Enable foreign keys @@ -8819,6 +8866,13 @@ class AppDatabase extends _$AppDatabase { // media row mapper reads it on every hydration. await _assertMediaManualElapsedColumn(); + // v169 backstop: re-assert dive_computers.equipment_id (gear twins; + // same parallel-branch version-collision self-heal). Column only: + // backfillDiveComputerGearTwins is a full-table pass that belongs to + // the ladder, and re-running it on every open would resurrect a gear + // item the user deleted. + await _assertDiveComputerEquipmentColumn(); + // v145 backstop: re-assert the gps_tracks provenance and trim columns. await _assertGpsTrackColumns(); diff --git a/lib/core/services/sync/sync_service.dart b/lib/core/services/sync/sync_service.dart index 5fd1999a7f..a212a4339b 100644 --- a/lib/core/services/sync/sync_service.dart +++ b/lib/core/services/sync/sync_service.dart @@ -2057,6 +2057,12 @@ class SyncService { (field: 'computerId', parent: 'diveComputers', nullable: true), (field: 'sourceId', parent: 'diveDataSources', nullable: true), ], + // v168 gear twins: a peer's live computer whose gear item we deleted + // locally would otherwise dangle this FK and abort the whole sync at + // COMMIT. Nullable, so the computer survives with the reference cleared. + 'diveComputers': [ + (field: 'equipmentId', parent: 'equipment', nullable: true), + ], 'diveTanks': [ (field: 'diveId', parent: 'dives', nullable: false), (field: 'equipmentId', parent: 'equipment', nullable: true), diff --git a/test/core/database/migration_v169_dive_computer_gear_test.dart b/test/core/database/migration_v169_dive_computer_gear_test.dart new file mode 100644 index 0000000000..9a1bb6031f --- /dev/null +++ b/test/core/database/migration_v169_dive_computer_gear_test.dart @@ -0,0 +1,105 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; + +/// v169 adds `dive_computers.equipment_id`: the equipment row that represents a +/// registered computer as gear, so a downloaded dive lists the computer that +/// logged it alongside the rest of the diver's kit. Nullable with no default, +/// because a cleared value means the user deleted that gear item and it must +/// not come back. +const String _preV169DiveComputers = ''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) +'''; + +NativeDatabase _dbAt168() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute(_preV169DiveComputers); + rawDb.execute( + "INSERT INTO dive_computers (id, name, created_at, updated_at) " + "VALUES ('c1', 'My Perdix', 1, 1)", + ); + }, + ); +} + +void main() { + test('v169 is in the migration ladder', () { + expect(AppDatabase.currentSchemaVersion, greaterThanOrEqualTo(169)); + expect(AppDatabase.migrationVersions, contains(169)); + }); + + test('a fresh database has dive_computers.equipment_id', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('equipment_id')); + }); + + test('the column is nullable and carries no default', () async { + final db = AppDatabase(NativeDatabase.memory()); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final column = cols.firstWhere( + (c) => c.read('name') == 'equipment_id', + ); + // A non-null default would claim every registered computer already has a + // gear item, and would resurrect one the user deleted. + expect(column.read('notnull'), 0); + expect(column.read('dflt_value'), isNull); + }); + + test('a database at v168 gains the column and keeps its rows', () async { + final db = AppDatabase(_dbAt168()); + addTearDown(db.close); + + final row = await db + .customSelect( + "SELECT name, equipment_id FROM dive_computers WHERE id = 'c1'", + ) + .getSingle(); + expect(row.read('name'), 'My Perdix'); + expect(row.read('equipment_id'), isNull); + }); + + test('a database stranded at a parallel-branch v169 gains the column via ' + 'beforeOpen', () async { + // Stamped AT 169 but without the column: the onUpgrade block never runs, + // so only the beforeOpen backstop can add it. + final nativeDb = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 169'); + rawDb.execute(_preV169DiveComputers); + }, + ); + final db = AppDatabase(nativeDb); + addTearDown(db.close); + + final cols = await db + .customSelect("PRAGMA table_info('dive_computers')") + .get(); + final names = cols.map((c) => c.read('name')).toSet(); + expect(names, contains('equipment_id')); + }); +} From 402fadaf81ce2effaf185fce285e1a46d2a23a4e Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:17:13 -0400 Subject: [PATCH 05/19] feat(equipment): seed a gear twin when a dive computer is registered --- .../dive_computer_repository_impl.dart | 22 +++ .../domain/entities/dive_computer.dart | 10 ++ .../services/dive_computer_gear_resolver.dart | 112 ++++++++++++ .../dive_computer_gear_resolver_test.dart | 170 ++++++++++++++++++ 4 files changed, 314 insertions(+) create mode 100644 lib/features/equipment/data/services/dive_computer_gear_resolver.dart create mode 100644 test/features/equipment/data/services/dive_computer_gear_resolver_test.dart diff --git a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart index 34ff57adb8..b96a7ab229 100644 --- a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart +++ b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart @@ -26,6 +26,7 @@ import 'package:submersion/features/dive_sites/domain/entities/dive_site.dart' show GeoPoint; import 'package:submersion/features/dive_log/domain/services/bottom_time_calculator.dart'; import 'package:submersion/features/dive_log/domain/services/dive_altitude_enricher.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_resolver.dart'; import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; import 'package:submersion/features/pre_dive/data/services/checklist_dive_linker.dart'; import 'package:submersion/core/services/database_service.dart'; @@ -252,6 +253,25 @@ class DiveComputerRepository { localUpdatedAt: now, ); + // Seed the gear twin once, here, because this is the only repository + // path that genuinely inserts a registry row (v169). Minting nowhere + // else is what makes a user-deleted twin permanent. Pass the resolved + // id: the caller's may have been empty and minted just above. + final twinId = await DiveComputerGearResolver().resolveGearTwin( + computer.copyWith(id: id), + ); + if (twinId != null) { + await _db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, id], + ); + await _syncRepository.markRecordPending( + entityType: 'diveComputers', + recordId: id, + localUpdatedAt: now, + ); + } + // If a computer with this hardware identity was deleted earlier, its // dives kept provenance snapshots; give them their link back. await _relinkOrphanedRows(id, computer); @@ -260,6 +280,7 @@ class DiveComputerRepository { _log.info('Created dive computer with id: $id'); return computer.copyWith( id: id, + equipmentId: twinId, createdAt: DateTime.fromMillisecondsSinceEpoch(now), updatedAt: DateTime.fromMillisecondsSinceEpoch(now), ); @@ -1949,6 +1970,7 @@ class DiveComputerRepository { diveCount: row.diveCount, isFavorite: row.isFavorite, notes: row.notes, + equipmentId: row.equipmentId, createdAt: DateTime.fromMillisecondsSinceEpoch(row.createdAt), updatedAt: DateTime.fromMillisecondsSinceEpoch(row.updatedAt), ); diff --git a/lib/features/dive_log/domain/entities/dive_computer.dart b/lib/features/dive_log/domain/entities/dive_computer.dart index 125f018e55..0b1bfc8d86 100644 --- a/lib/features/dive_log/domain/entities/dive_computer.dart +++ b/lib/features/dive_log/domain/entities/dive_computer.dart @@ -47,6 +47,12 @@ class DiveComputer extends Equatable { /// Additional notes final String notes; + /// The equipment row representing this device as gear, its gear twin (v169). + /// + /// Null when the user has deleted that gear item, which is permanent: only a + /// genuine computer registration mints a twin, so nothing re-creates it. + final String? equipmentId; + /// When this record was created final DateTime createdAt; @@ -68,6 +74,7 @@ class DiveComputer extends Equatable { this.diveCount = 0, this.isFavorite = false, this.notes = '', + this.equipmentId, required this.createdAt, required this.updatedAt, }); @@ -128,6 +135,7 @@ class DiveComputer extends Equatable { int? diveCount, bool? isFavorite, String? notes, + String? equipmentId, DateTime? createdAt, DateTime? updatedAt, }) { @@ -146,6 +154,7 @@ class DiveComputer extends Equatable { diveCount: diveCount ?? this.diveCount, isFavorite: isFavorite ?? this.isFavorite, notes: notes ?? this.notes, + equipmentId: equipmentId ?? this.equipmentId, createdAt: createdAt ?? this.createdAt, updatedAt: updatedAt ?? this.updatedAt, ); @@ -167,6 +176,7 @@ class DiveComputer extends Equatable { diveCount, isFavorite, notes, + equipmentId, createdAt, updatedAt, ]; diff --git a/lib/features/equipment/data/services/dive_computer_gear_resolver.dart b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart new file mode 100644 index 0000000000..20c7d35f30 --- /dev/null +++ b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart @@ -0,0 +1,112 @@ +import 'package:drift/drift.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/data/repositories/sync_repository.dart'; +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/logger_service.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart' + as domain; + +/// Resolves the equipment row that represents a registered dive computer as +/// gear, creating one if the diver does not already have a suitable item. +/// +/// Called only where a `dive_computers` row is genuinely created. That single +/// rule is what makes deleting a gear twin permanent: nothing else mints, so a +/// cleared `dive_computers.equipment_id` stays cleared. +class DiveComputerGearResolver { + DiveComputerGearResolver({SyncRepository? syncRepository}) + : _syncRepository = syncRepository ?? SyncRepository(); + + final SyncRepository _syncRepository; + final _log = LoggerService.forClass(DiveComputerGearResolver); + + AppDatabase get _db => DatabaseService.instance.database; + + /// The equipment id representing [computer], minting one when needed. + /// + /// Resolution order, which is the design: + /// 1. the stored link, when its equipment row still exists + /// 2. the row already holding the derived id, which survives a rename + /// 3. exactly one unambiguous identity match among active computer gear + /// 4. mint at the derived id + /// + /// Returns null and logs on failure. A computer that fails to get a twin is + /// still a correctly registered computer, so registration must not fail + /// because gear seeding did. + Future resolveGearTwin(domain.DiveComputer computer) async { + try { + final stored = computer.equipmentId; + if (stored != null && stored.isNotEmpty) { + final existing = await (_db.select( + _db.equipment, + )..where((t) => t.id.equals(stored))).getSingleOrNull(); + if (existing != null) return stored; + } + + final derivedId = diveComputerGearId(computer.id); + + // The identity match below reads each row's CURRENT text while the id + // derives from the computer id, so a renamed gear item makes the match + // miss while the id still collides. Adopt the row holding it rather than + // letting the insert throw SqliteException(1555). + final byDerivedId = await (_db.select( + _db.equipment, + )..where((t) => t.id.equals(derivedId))).getSingleOrNull(); + if (byDerivedId != null) return derivedId; + + final rows = + await (_db.select(_db.equipment) + ..where((t) => t.type.equals(EquipmentType.computer.name)) + ..where((t) => t.isActive.equals(true))) + .get(); + final match = matchGearTwin( + manufacturer: computer.manufacturer, + model: computer.model, + serialNumber: computer.serialNumber, + diverId: computer.diverId, + candidates: rows.map( + (r) => GearTwinCandidate( + id: r.id, + diverId: r.diverId, + brand: r.brand, + model: r.model, + serialNumber: r.serialNumber, + ), + ), + ); + if (match != null) return match.id; + + final now = DateTime.now().millisecondsSinceEpoch; + await _db + .into(_db.equipment) + .insertOnConflictUpdate( + EquipmentCompanion.insert( + id: derivedId, + diverId: Value(computer.diverId), + name: computer.name, + type: EquipmentType.computer.name, + brand: Value(computer.manufacturer), + model: Value(computer.model), + serialNumber: Value(computer.serialNumber), + createdAt: now, + updatedAt: now, + ), + ); + await _syncRepository.markRecordPending( + entityType: 'equipment', + recordId: derivedId, + localUpdatedAt: now, + ); + return derivedId; + } catch (e, stackTrace) { + _log.error( + 'Failed to resolve a gear twin for computer ${computer.id}', + error: e, + stackTrace: stackTrace, + ); + return null; + } + } +} diff --git a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart new file mode 100644 index 0000000000..21966dbb52 --- /dev/null +++ b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart @@ -0,0 +1,170 @@ +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart' hide DiveComputer; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_resolver.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late DiveComputerGearResolver resolver; + + setUp(() async { + db = await setUpTestDatabase(); + // Equipment writes without full Diver fixtures. + await db.customStatement('PRAGMA foreign_keys = OFF'); + resolver = DiveComputerGearResolver(); + }); + tearDown(tearDownTestDatabase); + + DiveComputer computer({ + String id = 'c1', + String? diverId = 'd1', + String name = 'My Perdix', + String? manufacturer = 'Shearwater', + String? model = 'Perdix 2', + String? serialNumber, + String? equipmentId, + }) => DiveComputer( + id: id, + diverId: diverId, + name: name, + manufacturer: manufacturer, + model: model, + serialNumber: serialNumber, + equipmentId: equipmentId, + createdAt: DateTime.now(), + updatedAt: DateTime.now(), + ); + + Future insertGear( + String id, { + String? diverId = 'd1', + String type = 'computer', + String? brand, + String? model, + String? serialNumber, + bool isActive = true, + }) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db + .into(db.equipment) + .insert( + EquipmentCompanion.insert( + id: id, + diverId: Value(diverId), + name: id, + type: type, + brand: Value(brand), + model: Value(model), + serialNumber: Value(serialNumber), + isActive: Value(isActive), + createdAt: t, + updatedAt: t, + ), + ); + } + + Future equipmentCount() async { + final row = await db + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + return row.read('c'); + } + + test('mints a twin at the deterministic id when nothing matches', () async { + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + final row = await (db.select( + db.equipment, + )..where((t) => t.id.equals(id!))).getSingle(); + expect(row.type, 'computer'); + expect(row.name, 'My Perdix'); + expect(row.brand, 'Shearwater'); + expect(row.model, 'Perdix 2'); + // Seeded once, then owned by the user: service fields stay theirs to set. + expect(row.purchaseDate, isNull); + expect(row.serviceIntervalDays, isNull); + }); + + test('returns the stored link when its equipment row still exists', () async { + await insertGear('hand-made'); + + final id = await resolver.resolveGearTwin( + computer(equipmentId: 'hand-made'), + ); + + expect(id, 'hand-made'); + }); + + test('mints when the stored link points at a deleted row', () async { + final id = await resolver.resolveGearTwin(computer(equipmentId: 'gone')); + + expect(id, diveComputerGearId('c1')); + }); + + test('adopts the row holding the derived id after a rename', () async { + // The identity match reads the row's CURRENT text while the id derives + // from the computer id, so renaming makes the match miss while the id + // still collides. Without this branch the insert throws + // SqliteException(1555) UNIQUE constraint failed. + await insertGear( + diveComputerGearId('c1'), + brand: 'Totally', + model: 'Renamed', + ); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + expect(await equipmentCount(), 1); + }); + + test('adopts an unambiguous hand-created gear item', () async { + await insertGear('hand-made', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, 'hand-made'); + }); + + test('mints rather than guessing between two identical candidates', () async { + await insertGear('one', brand: 'Shearwater', model: 'Perdix 2'); + await insertGear('two', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + }); + + test('ignores retired gear and non-computer gear when matching', () async { + await insertGear( + 'retired', + brand: 'Shearwater', + model: 'Perdix 2', + isActive: false, + ); + await insertGear( + 'a-bcd', + type: 'bcd', + brand: 'Shearwater', + model: 'Perdix 2', + ); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, diveComputerGearId('c1')); + }); + + test('is idempotent across repeated calls', () async { + final first = await resolver.resolveGearTwin(computer()); + final second = await resolver.resolveGearTwin(computer()); + + expect(first, second); + expect(await equipmentCount(), 1); + }); +} From 7a7b64e0a2946b4496b8e6424f04ec5dfe5b8cc6 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:22:23 -0400 Subject: [PATCH 06/19] feat(equipment): attach dive computer gear twins at the import seams Adds DiveComputerGearLinker, a fourth member of the existing defaulter / checklist-linker / altitude-enricher trio, wired at all four non-interactive creation seams. Link-only: it never mints equipment, which is what makes a user-deleted twin permanent. The ordering test is the point of the task, not decoration. The defaulter bails when a dive already has any dive_equipment row, so a linker running first silently suppresses the diver's default and geofenced sets. Both directions are asserted, so the constraint fails loudly if anyone reorders the calls. Does NOT reuse DiveComputerRepository.getComputerIdsForDive, which the design had called for. That method reads dive_profiles, so it sees only dives with profile samples; a file-imported dive registered by #1288 can have computer_id stamped and a data-source row with no samples at all, and reusing it would have silently failed to link exactly the file-import case. The linker owns a query over the union of dive_data_sources.computer_id and dives.computer_id, the same union the v169 backfill uses. Design doc corrected. --- ...26-08-26-dive-computer-gear-twin-design.md | 17 ++- .../data/services/uddf_entity_importer.dart | 5 + .../providers/dive_import_providers.dart | 5 + .../dive_computer_repository_impl.dart | 6 + .../services/dive_computer_gear_linker.dart | 91 ++++++++++++ .../data/adapters/healthkit_adapter.dart | 5 + .../dive_computer_gear_linker_test.dart | 133 ++++++++++++++++++ .../gear_twin_defaulter_ordering_test.dart | 99 +++++++++++++ 8 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 lib/features/equipment/data/services/dive_computer_gear_linker.dart create mode 100644 test/features/equipment/data/services/dive_computer_gear_linker_test.dart create mode 100644 test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md index dca31206cd..e5f8f73f71 100644 --- a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -278,10 +278,19 @@ Future linkComputerGearForDive({ }); ``` -It reads the dive's computers via the existing `getComputerIdsForDive` (which -reads `dive_data_sources`, per F2, so a multi-source dive links every computer -that logged it), takes each computer's `equipmentId` **as stored**, and attaches -the non-null ones via `bulkAddEquipment`. +It resolves the dive's computers from the union of `dive_data_sources.computer_id` +and `dives.computer_id` (per F2, so a multi-source dive links every computer that +logged it), takes each computer's `equipmentId` **as stored**, and attaches the +non-null ones via `bulkAddEquipment`. + +**Correction, found in implementation.** An earlier draft of this section said to +reuse `DiveComputerRepository.getComputerIdsForDive`. That method reads +`dive_profiles`, not `dive_data_sources`, so it sees only dives carrying profile +samples. A file-imported dive registered by #1288 can have `computer_id` stamped +and a data-source row while having no samples at all, and reusing the helper +would have silently failed to link exactly the file-import case this feature was +extended to cover. The linker owns a private query applying the same union the +v169 backfill uses, so the migration and the runtime path cannot disagree. It performs no resolution and no minting. That is what makes deletion permanent: creation happens once at registration, linking happens per dive, and a cleared diff --git a/lib/features/dive_import/data/services/uddf_entity_importer.dart b/lib/features/dive_import/data/services/uddf_entity_importer.dart index 7194f6fe3f..d2ec1db13f 100644 --- a/lib/features/dive_import/data/services/uddf_entity_importer.dart +++ b/lib/features/dive_import/data/services/uddf_entity_importer.dart @@ -5,6 +5,7 @@ import 'package:submersion/core/database/database.dart' show DiveDataSourcesCompanion, DiveSitesCompanion, DivesCompanion; import 'package:submersion/core/services/export/export_service.dart'; import 'package:submersion/features/dive_log/domain/services/dive_altitude_enricher.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; import 'package:submersion/features/pre_dive/data/services/checklist_dive_linker.dart'; import 'package:submersion/core/services/location_service.dart'; @@ -1719,6 +1720,10 @@ class UddfEntityImporter { await DiveEquipmentDefaulter().applyForImportedDive(dive); await ChecklistDiveLinker().applyForImportedDive(dive); await altitudeEnricher.applyForImportedDive(dive); + // After the defaulter, never before: the defaulter bails on a dive + // that already has equipment, so linking first would suppress the + // diver's default and geofenced sets. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: dive.id); importedDiveIds.add(diveId); diveIdByIndex[i] = diveId; diff --git a/lib/features/dive_import/presentation/providers/dive_import_providers.dart b/lib/features/dive_import/presentation/providers/dive_import_providers.dart index 375265606d..0b64b0f30d 100644 --- a/lib/features/dive_import/presentation/providers/dive_import_providers.dart +++ b/lib/features/dive_import/presentation/providers/dive_import_providers.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:submersion/core/providers/provider.dart'; import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; import 'package:submersion/features/dive_log/domain/services/dive_altitude_enricher.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; import 'package:submersion/features/pre_dive/data/services/checklist_dive_linker.dart'; import 'package:submersion/features/dive_import/data/services/fit_parser_service.dart'; @@ -396,6 +397,10 @@ class DiveImportNotifier extends StateNotifier { await DiveEquipmentDefaulter().applyForImportedDive(dive); await ChecklistDiveLinker().applyForImportedDive(dive); await altitudeEnricher.applyForImportedDive(dive); + // After the defaulter, never before: the defaulter bails on a dive + // that already has equipment, so linking first would suppress the + // diver's default and geofenced sets. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: dive.id); imported++; } diff --git a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart index b96a7ab229..42e316f381 100644 --- a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart +++ b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart @@ -26,6 +26,7 @@ import 'package:submersion/features/dive_sites/domain/entities/dive_site.dart' show GeoPoint; import 'package:submersion/features/dive_log/domain/services/bottom_time_calculator.dart'; import 'package:submersion/features/dive_log/domain/services/dive_altitude_enricher.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; import 'package:submersion/features/equipment/data/services/dive_computer_gear_resolver.dart'; import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; import 'package:submersion/features/pre_dive/data/services/checklist_dive_linker.dart'; @@ -1229,6 +1230,11 @@ class DiveComputerRepository { divePoints: defaultPoints, ); + // After the defaulter, never before: the defaulter bails on a dive + // that already has equipment, so linking first would suppress the + // diver's default and geofenced sets. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: diveId); + // Auto-link a pre-dive checklist session started shortly before // this dive's entry time. await ChecklistDiveLinker().autoLinkForDive( diff --git a/lib/features/equipment/data/services/dive_computer_gear_linker.dart b/lib/features/equipment/data/services/dive_computer_gear_linker.dart new file mode 100644 index 0000000000..cb9d4159da --- /dev/null +++ b/lib/features/equipment/data/services/dive_computer_gear_linker.dart @@ -0,0 +1,91 @@ +import 'package:drift/drift.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/services/database_service.dart'; +import 'package:submersion/core/services/sync/sync_event_bus.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; + +/// Attaches the gear twins of the dive computers that logged a dive (v169). +/// +/// Used by the non-interactive creation seams (dive-computer download, file +/// import), alongside `DiveEquipmentDefaulter`, `ChecklistDiveLinker` and +/// `DiveAltitudeEnricher`. +/// +/// Link-only: it never creates an equipment row. Creation happens once, at +/// computer registration, so a twin the user deleted (which clears +/// `dive_computers.equipment_id`) simply produces no link and stays deleted. +class DiveComputerGearLinker { + DiveComputerGearLinker({DiveRepository? diveRepository}) + : _dives = diveRepository ?? DiveRepository(); + + final DiveRepository _dives; + + AppDatabase get _db => DatabaseService.instance.database; + + /// Returns true when at least one twin was attached. + /// + /// MUST run after `DiveEquipmentDefaulter` at every seam: the defaulter bails + /// when the dive already has any `dive_equipment` row, so linking first would + /// silently suppress the diver's default and geofenced equipment sets. + /// + /// Unlike the defaulter this is NOT gated on the dive being empty: the + /// computer belongs on the dive whether or not a set already applied. + /// + /// Best-effort: any failure is swallowed so equipment linking can never abort + /// a download or import that has already persisted the dive. + Future linkComputerGearForDive({required String diveId}) async { + if (DatabaseService.instance.databaseOrNull == null) return false; + try { + final computerIds = await _computerIdsForDive(diveId); + if (computerIds.isEmpty) return false; + + final rows = await (_db.select( + _db.diveComputers, + )..where((t) => t.id.isIn(computerIds))).get(); + final equipmentIds = rows + .map((r) => r.equipmentId) + .whereType() + .where((id) => id.isNotEmpty) + .toSet() + .toList(); + if (equipmentIds.isEmpty) return false; + + await _dives.bulkAddEquipment([diveId], equipmentIds); + SyncEventBus.notifyLocalChange(); + return true; + } catch (_) { + // Best-effort: never let gear linking fail the dive operation. + return false; + } + } + + /// Every computer that logged [diveId]. + /// + /// Deliberately NOT `DiveComputerRepository.getComputerIdsForDive`, which + /// reads `dive_profiles` and so sees only dives that carry profile samples. + /// A file-imported dive registered by #1288 can have `computer_id` stamped + /// and a data-source row while having no samples at all, and its computer + /// belongs on it just the same. + /// + /// `dives.computer_id` alone is not enough either: it holds only the primary, + /// so a dive logged on two computers would list one. This is the same union + /// the v169 backfill applies, so the migration and the runtime path agree. + Future> _computerIdsForDive(String diveId) async { + final rows = await _db + .customSelect( + 'SELECT DISTINCT computer_id FROM (' + ' SELECT computer_id FROM dive_data_sources ' + ' WHERE dive_id = ? AND computer_id IS NOT NULL' + ' UNION' + ' SELECT computer_id FROM dives ' + ' WHERE id = ? AND computer_id IS NOT NULL' + ')', + variables: [Variable(diveId), Variable(diveId)], + ) + .get(); + return rows + .map((row) => row.data['computer_id'] as String?) + .whereType() + .toList(); + } +} diff --git a/lib/features/import_wizard/data/adapters/healthkit_adapter.dart b/lib/features/import_wizard/data/adapters/healthkit_adapter.dart index 75e71649a3..87025295a2 100644 --- a/lib/features/import_wizard/data/adapters/healthkit_adapter.dart +++ b/lib/features/import_wizard/data/adapters/healthkit_adapter.dart @@ -7,6 +7,7 @@ import 'package:submersion/core/utils/unit_formatter.dart'; import 'package:submersion/features/data_quality/data/services/quality_scan_service.dart'; import 'package:submersion/features/dive_import/domain/entities/imported_dive.dart'; import 'package:submersion/features/dive_log/domain/services/dive_altitude_enricher.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; import 'package:submersion/features/pre_dive/data/services/checklist_dive_linker.dart'; import 'package:submersion/features/dive_import/domain/services/dive_matcher.dart'; @@ -283,6 +284,10 @@ class HealthKitAdapter implements ImportSourceAdapter { await DiveEquipmentDefaulter().applyForImportedDive(dive); await ChecklistDiveLinker().applyForImportedDive(dive); await altitudeEnricher.applyForImportedDive(dive); + // After the defaulter, never before: the defaulter bails on a dive + // that already has equipment, so linking first would suppress the + // diver's default and geofenced sets. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: dive.id); imported++; importedDiveIds.add(dive.id); diff --git a/test/features/equipment/data/services/dive_computer_gear_linker_test.dart b/test/features/equipment/data/services/dive_computer_gear_linker_test.dart new file mode 100644 index 0000000000..5d7fd65bc0 --- /dev/null +++ b/test/features/equipment/data/services/dive_computer_gear_linker_test.dart @@ -0,0 +1,133 @@ +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; + +import '../../../../helpers/test_database.dart'; + +void main() { + late AppDatabase db; + late DiveComputerGearLinker linker; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + linker = DiveComputerGearLinker(); + }); + tearDown(tearDownTestDatabase); + + Future insertGear(String id) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db + .into(db.equipment) + .insert( + EquipmentCompanion.insert( + id: id, + name: id, + type: 'computer', + createdAt: t, + updatedAt: t, + ), + ); + } + + Future insertComputer(String id, {String? equipmentId}) async { + final t = DateTime.now().millisecondsSinceEpoch; + await db + .into(db.diveComputers) + .insert( + DiveComputersCompanion.insert( + id: id, + name: id, + equipmentId: Value(equipmentId), + createdAt: t, + updatedAt: t, + ), + ); + } + + Future linkSource(String diveId, String computerId) async { + await db.customStatement( + 'INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, ' + 'imported_at, created_at) VALUES (?, ?, ?, 1, 1, 1)', + ['src-$diveId-$computerId', diveId, computerId], + ); + } + + Future> equipmentOn(String diveId) async { + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).get(); + return rows.map((r) => r.equipmentId).toSet(); + } + + test('attaches the gear twin of the computer that logged the dive', () async { + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'gear-1'}); + }); + + test('attaches every computer on a multi-source dive', () async { + // dives.computer_id holds only the primary; a twin-computer diver must get + // both, which is why the linker reads dive_data_sources. + await insertGear('gear-1'); + await insertGear('gear-2'); + await insertComputer('c1', equipmentId: 'gear-1'); + await insertComputer('c2', equipmentId: 'gear-2'); + await linkSource('dive1', 'c1'); + await linkSource('dive1', 'c2'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'gear-1', 'gear-2'}); + }); + + test('adds to existing equipment rather than replacing it', () async { + // Unlike the defaulter, the linker is not gated on the dive being empty. + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + await db + .into(db.diveEquipment) + .insert( + DiveEquipmentCompanion.insert(diveId: 'dive1', equipmentId: 'a-bcd'), + ); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isTrue); + expect(await equipmentOn('dive1'), {'a-bcd', 'gear-1'}); + }); + + test( + 'never creates equipment for a computer whose twin was deleted', + () async { + await insertComputer('c1'); + await linkSource('dive1', 'c1'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isFalse); + expect(await equipmentOn('dive1'), isEmpty); + final count = await db + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + expect(count.read('c'), 0); + }, + ); + + test('is a no-op for a dive with no registered computer', () async { + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isFalse); + expect(await equipmentOn('dive1'), isEmpty); + }); + + test('is idempotent', () async { + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + + await linker.linkComputerGearForDive(diveId: 'dive1'); + await linker.linkComputerGearForDive(diveId: 'dive1'); + + expect(await equipmentOn('dive1'), {'gear-1'}); + }); +} diff --git a/test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart b/test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart new file mode 100644 index 0000000000..3f5995f850 --- /dev/null +++ b/test/features/equipment/data/services/gear_twin_defaulter_ordering_test.dart @@ -0,0 +1,99 @@ +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart' hide EquipmentSet; +import 'package:submersion/features/equipment/data/repositories/equipment_set_repository_impl.dart'; +import 'package:submersion/features/equipment/data/services/dive_computer_gear_linker.dart'; +import 'package:submersion/features/equipment/data/services/dive_equipment_defaulter.dart'; +import 'package:submersion/features/equipment/domain/entities/equipment_set.dart'; + +import '../../../../helpers/test_database.dart'; + +/// The defaulter bails when the dive already has any dive_equipment row, so +/// running the gear linker FIRST would silently suppress the diver's default +/// and geofenced equipment sets. A downloaded dive must receive both. +void main() { + late AppDatabase db; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + final t = DateTime.now().millisecondsSinceEpoch; + for (final id in ['a-bcd', 'gear-1']) { + await db + .into(db.equipment) + .insert( + EquipmentCompanion.insert( + id: id, + name: id, + type: id == 'gear-1' ? 'computer' : 'bcd', + createdAt: t, + updatedAt: t, + ), + ); + } + await db + .into(db.diveComputers) + .insert( + DiveComputersCompanion.insert( + id: 'c1', + name: 'c1', + equipmentId: const Value('gear-1'), + createdAt: t, + updatedAt: t, + ), + ); + await db.customStatement( + "INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, " + "imported_at, created_at) VALUES ('s1', 'dive1', 'c1', 1, 1, 1)", + ); + final sets = EquipmentSetRepository(); + await sets.createSet( + EquipmentSet( + id: 'def', + diverId: 'd1', + name: 'def', + equipmentIds: const ['a-bcd'], + isDefault: true, + createdAt: DateTime.now(), + updatedAt: DateTime.now(), + ), + ); + await sets.setAsDefault('def', diverId: 'd1'); + }); + tearDown(tearDownTestDatabase); + + Future> equipmentOn(String diveId) async { + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).get(); + return rows.map((r) => r.equipmentId).toSet(); + } + + test('defaulter first, then linker: the dive gets BOTH', () async { + await DiveEquipmentDefaulter().applyDefaultEquipmentIfEmpty( + diveId: 'dive1', + diverId: 'd1', + divePoints: const [], + ); + await DiveComputerGearLinker().linkComputerGearForDive(diveId: 'dive1'); + + expect(await equipmentOn('dive1'), {'a-bcd', 'gear-1'}); + }); + + test( + 'linker first would suppress the default set, proving the order', + () async { + await DiveComputerGearLinker().linkComputerGearForDive(diveId: 'dive1'); + final applied = await DiveEquipmentDefaulter() + .applyDefaultEquipmentIfEmpty( + diveId: 'dive1', + diverId: 'd1', + divePoints: const [], + ); + + expect(applied, isFalse); + expect(await equipmentOn('dive1'), {'gear-1'}); + }, + ); +} From 5a2bdc481333b96febd83eb36ecbeb79c01301a5 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:23:30 -0400 Subject: [PATCH 07/19] feat(equipment): link the gear twin when a dive source is replaced --- .../dive_computer_repository_impl.dart | 7 ++ .../replace_source_gear_link_test.dart | 83 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 test/features/dive_log/data/repositories/replace_source_gear_link_test.dart diff --git a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart index 42e316f381..ced50a2240 100644 --- a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart +++ b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart @@ -1614,6 +1614,13 @@ class DiveComputerRepository { recordId: diveId, localUpdatedAt: now, ); + + // The replaceSource path clears this dive's data source on the way in + // and importProfile re-creates it above, so the linker can see this + // computer again by here. The creation-seam trio does not run for an + // existing dive, but the computer did log it. Idempotent through + // insertOnConflictUpdate. + await DiveComputerGearLinker().linkComputerGearForDive(diveId: diveId); } // Note: Computer stats (incrementDiveCount, updateLastDownload) are diff --git a/test/features/dive_log/data/repositories/replace_source_gear_link_test.dart b/test/features/dive_log/data/repositories/replace_source_gear_link_test.dart new file mode 100644 index 0000000000..7598221e25 --- /dev/null +++ b/test/features/dive_log/data/repositories/replace_source_gear_link_test.dart @@ -0,0 +1,83 @@ +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_computer_repository_impl.dart'; + +import '../../../../helpers/test_database.dart'; + +/// A replaceSource re-download matches an existing dive, so importProfile takes +/// the isNewDive == false branch and the creation-seam trio never runs. The +/// computer still logged the dive, so its gear twin belongs on it. +void main() { + late AppDatabase db; + late DiveComputerRepository repo; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + repo = DiveComputerRepository(); + final t = DateTime.now().millisecondsSinceEpoch; + await db + .into(db.equipment) + .insert( + EquipmentCompanion.insert( + id: 'gear-1', + name: 'gear-1', + type: 'computer', + createdAt: t, + updatedAt: t, + ), + ); + await db + .into(db.diveComputers) + .insert( + DiveComputersCompanion.insert( + id: 'c1', + name: 'c1', + equipmentId: const Value('gear-1'), + createdAt: t, + updatedAt: t, + ), + ); + }); + tearDown(tearDownTestDatabase); + + Future> equipmentOn(String diveId) async { + final rows = await (db.select( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).get(); + return rows.map((r) => r.equipmentId).toSet(); + } + + test('re-importing onto an existing dive links the gear twin', () async { + final start = DateTime.fromMillisecondsSinceEpoch(1700000000000); + + final diveId = await repo.importProfile( + computerId: 'c1', + profileStartTime: start, + points: const [], + durationSeconds: 1800, + maxDepth: 30.0, + ); + + // Remove the link so the second pass has something to prove. + await (db.delete( + db.diveEquipment, + )..where((t) => t.diveId.equals(diveId))).go(); + await repo.clearSourceAndProfiles(diveId: diveId, computerId: 'c1'); + expect(await equipmentOn(diveId), isEmpty); + + // Second import matches the same dive: the isNewDive == false branch. + final again = await repo.importProfile( + computerId: 'c1', + profileStartTime: start, + points: const [], + durationSeconds: 1800, + maxDepth: 30.0, + ); + + expect(again, diveId); + expect(await equipmentOn(diveId), contains('gear-1')); + }); +} From 9e0773ad9c3fb439771ad6cb749927be1e14d99d Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:26:13 -0400 Subject: [PATCH 08/19] feat(db): backfill dive computer gear twins at v169 --- lib/core/database/database.dart | 2 + .../database/dive_computer_gear_backfill.dart | 159 ++++++++++++ .../dive_computer_gear_backfill_test.dart | 245 ++++++++++++++++++ 3 files changed, 406 insertions(+) create mode 100644 lib/core/database/dive_computer_gear_backfill.dart create mode 100644 test/core/database/dive_computer_gear_backfill_test.dart diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index f145ef9d93..105e7de568 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -3,6 +3,7 @@ import 'dart:developer' as developer; import 'package:drift/drift.dart'; +import 'package:submersion/core/database/dive_computer_gear_backfill.dart'; import 'package:submersion/core/database/imported_computer_backfill.dart'; import 'package:submersion/core/database/performance_indexes.dart'; import 'package:submersion/core/database/tag_uniqueness.dart'; @@ -8660,6 +8661,7 @@ class AppDatabase extends _$AppDatabase { // a later step; the column has to land first. if (from < 169) { await _assertDiveComputerEquipmentColumn(); + await backfillDiveComputerGearTwins(this); } if (from < 169) await reportProgress(); }, diff --git a/lib/core/database/dive_computer_gear_backfill.dart b/lib/core/database/dive_computer_gear_backfill.dart new file mode 100644 index 0000000000..fcf7b56ec3 --- /dev/null +++ b/lib/core/database/dive_computer_gear_backfill.dart @@ -0,0 +1,159 @@ +import 'package:drift/drift.dart'; + +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// Seed a gear twin for every registered dive computer and link it to the dives +/// that computer logged (v169). +/// +/// Ladder-only, never a `beforeOpen` backstop, for two independent reasons: it +/// is a full-table pass over every dive, and re-running it on every open would +/// resurrect a gear item the user deleted. That is the same rule +/// `_backfillLegacyServiceSchedules` and `_backfillBottomTimeFromProfile` +/// follow. +/// +/// New rows land on a deterministic id ([diveComputerGearId]), so every device +/// in a synced fleet derives the same primary key and they converge under sync +/// upsert rather than duplicating. +Future backfillDiveComputerGearTwins(DatabaseConnectionUser db) async { + // PRAGMA-guarded like every other backfill helper: the ladder runs against + // minimal fixtures and against databases caught mid-upgrade. PRAGMA + // table_info returns empty for a missing table, so probing the columns covers + // both "table absent" and "column absent". + Future> columnsOf(String table) async { + final rows = await db.customSelect("PRAGMA table_info('$table')").get(); + return rows.map((c) => c.read('name')).toSet(); + } + + final computerCols = await columnsOf('dive_computers'); + if (!computerCols.containsAll({ + 'id', + 'diver_id', + 'name', + 'manufacturer', + 'model', + 'serial_number', + 'equipment_id', + })) { + return; + } + final equipmentCols = await columnsOf('equipment'); + if (!equipmentCols.containsAll({ + 'id', + 'diver_id', + 'name', + 'type', + 'brand', + 'model', + 'serial_number', + 'is_active', + 'created_at', + 'updated_at', + })) { + return; + } + + // Pass 1: resolve a twin per computer. Bounded by device count, a handful of + // rows, so no event-loop yield is needed here. + final computers = await db + .customSelect( + 'SELECT id, diver_id, name, manufacturer, model, serial_number ' + 'FROM dive_computers WHERE equipment_id IS NULL ORDER BY id', + ) + .get(); + + for (final computer in computers) { + final id = computer.read('id'); + final diverId = computer.read('diver_id'); + final name = computer.read('name'); + final manufacturer = computer.read('manufacturer'); + final model = computer.read('model'); + final serial = computer.read('serial_number'); + + final derivedId = diveComputerGearId(id); + + // Adopt the row already holding the derived id before matching on text: + // the match reads each row's CURRENT text while the id derives from the + // computer id, so a renamed gear item makes the match miss while the id + // still collides. + final byDerivedId = await db + .customSelect( + 'SELECT id FROM equipment WHERE id = ?', + variables: [Variable(derivedId)], + ) + .getSingleOrNull(); + + var twinId = byDerivedId?.read('id'); + + if (twinId == null) { + final candidateRows = await db + .customSelect( + 'SELECT id, diver_id, brand, model, serial_number FROM equipment ' + "WHERE type = 'computer' AND is_active = 1 " + 'ORDER BY updated_at DESC, id', + ) + .get(); + twinId = matchGearTwin( + manufacturer: manufacturer, + model: model, + serialNumber: serial, + diverId: diverId, + candidates: candidateRows.map( + (r) => GearTwinCandidate( + id: r.read('id'), + diverId: r.read('diver_id'), + brand: r.read('brand'), + model: r.read('model'), + serialNumber: r.read('serial_number'), + ), + ), + )?.id; + } + + if (twinId == null) { + twinId = derivedId; + final now = DateTime.now().millisecondsSinceEpoch; + await db.customStatement( + 'INSERT OR IGNORE INTO equipment ' + '(id, diver_id, name, type, brand, model, serial_number, status, ' + 'purchase_currency, notes, is_active, created_at, updated_at) ' + "VALUES (?, ?, ?, 'computer', ?, ?, ?, 'active', 'USD', '', 1, ?, ?)", + [twinId, diverId, name, manufacturer, model, serial, now, now], + ); + } + + await db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, id], + ); + } + + // Pass 2: link the dives. Set-based, so it needs no per-dive loop and no + // event-loop yield: a per-dive loop during a migration runs as one unbroken + // microtask chain and freezes the progress spinner. + final junctionCols = await columnsOf('dive_equipment'); + if (!junctionCols.containsAll({'dive_id', 'equipment_id'})) return; + + // dives.computer_id holds only the PRIMARY computer, so it is unioned with + // dive_data_sources: a dive logged on two computers must list both. + final diveCols = await columnsOf('dives'); + if (diveCols.contains('computer_id')) { + await db.customStatement(''' + INSERT OR IGNORE INTO dive_equipment (dive_id, equipment_id) + SELECT d.id, c.equipment_id + FROM dives d + JOIN dive_computers c ON c.id = d.computer_id + WHERE c.equipment_id IS NOT NULL + '''); + } + + final sourceCols = await columnsOf('dive_data_sources'); + if (sourceCols.containsAll({'dive_id', 'computer_id'})) { + await db.customStatement(''' + INSERT OR IGNORE INTO dive_equipment (dive_id, equipment_id) + SELECT s.dive_id, c.equipment_id + FROM dive_data_sources s + JOIN dive_computers c ON c.id = s.computer_id + WHERE c.equipment_id IS NOT NULL + '''); + } +} diff --git a/test/core/database/dive_computer_gear_backfill_test.dart b/test/core/database/dive_computer_gear_backfill_test.dart new file mode 100644 index 0000000000..802bda0d34 --- /dev/null +++ b/test/core/database/dive_computer_gear_backfill_test.dart @@ -0,0 +1,245 @@ +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_backfill.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// The v169 backfill mints a gear twin per registered computer and links it to +/// every dive that computer logged. The fixture is stamped at 168 so the ladder +/// runs the real migration. +NativeDatabase _seeded() { + return NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute(''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE dives ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + computer_id TEXT, + dive_date_time INTEGER NOT NULL DEFAULT 0 + ) + '''); + rawDb.execute(''' + CREATE TABLE dive_data_sources ( + id TEXT NOT NULL PRIMARY KEY, + dive_id TEXT NOT NULL, + computer_id TEXT, + is_primary INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL DEFAULT 0 + ) + '''); + rawDb.execute(''' + CREATE TABLE dive_equipment ( + dive_id TEXT NOT NULL, + equipment_id TEXT NOT NULL, + PRIMARY KEY (dive_id, equipment_id) + ) + '''); + rawDb.execute(''' + CREATE TABLE equipment ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + type TEXT NOT NULL, + brand TEXT, + model TEXT, + serial_number TEXT, + status TEXT NOT NULL DEFAULT 'active', + purchase_currency TEXT NOT NULL DEFAULT 'USD', + notes TEXT NOT NULL DEFAULT '', + is_active INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + + rawDb.execute( + "INSERT INTO dive_computers (id, diver_id, name, manufacturer, model, " + "created_at, updated_at) VALUES " + "('c1', 'd1', 'My Perdix', 'Shearwater', 'Perdix 2', 1, 1)", + ); + rawDb.execute( + "INSERT INTO dive_computers (id, diver_id, name, manufacturer, model, " + "created_at, updated_at) VALUES " + "('c2', 'd1', 'My NERD', 'Shearwater', 'NERD 2', 1, 1)", + ); + // dive1: primary c1 only. dive2: two sources, c1 primary and c2. + rawDb.execute( + "INSERT INTO dives (id, diver_id, computer_id) " + "VALUES ('dive1', 'd1', 'c1')", + ); + rawDb.execute( + "INSERT INTO dives (id, diver_id, computer_id) " + "VALUES ('dive2', 'd1', 'c1')", + ); + rawDb.execute( + "INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, " + "created_at) VALUES ('s1', 'dive1', 'c1', 1, 1)", + ); + rawDb.execute( + "INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, " + "created_at) VALUES ('s2', 'dive2', 'c1', 1, 1)", + ); + rawDb.execute( + "INSERT INTO dive_data_sources (id, dive_id, computer_id, is_primary, " + "created_at) VALUES ('s3', 'dive2', 'c2', 0, 1)", + ); + }, + ); +} + +Future> _equipmentOn(AppDatabase db, String diveId) async { + final rows = await db + .customSelect( + "SELECT equipment_id FROM dive_equipment WHERE dive_id = '$diveId'", + ) + .get(); + return rows.map((r) => r.read('equipment_id')).toSet(); +} + +void main() { + test('mints a twin per computer and links its dives', () async { + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + final c1Twin = diveComputerGearId('c1'); + final c2Twin = diveComputerGearId('c2'); + + final computers = await db + .customSelect('SELECT id, equipment_id FROM dive_computers ORDER BY id') + .get(); + expect(computers[0].read('equipment_id'), c1Twin); + expect(computers[1].read('equipment_id'), c2Twin); + + expect(await _equipmentOn(db, 'dive1'), {c1Twin}); + // A multi-source dive gets BOTH computers: dives.computer_id holds only + // the primary, so the union with dive_data_sources is what catches c2. + expect(await _equipmentOn(db, 'dive2'), {c1Twin, c2Twin}); + }); + + test( + 'minted twins are computer-type gear carrying the device identity', + () async { + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + final row = await db + .customSelect( + "SELECT name, type, brand, model FROM equipment " + "WHERE id = '${diveComputerGearId('c1')}'", + ) + .getSingle(); + expect(row.read('type'), 'computer'); + expect(row.read('name'), 'My Perdix'); + expect(row.read('brand'), 'Shearwater'); + expect(row.read('model'), 'Perdix 2'); + }, + ); + + test('is idempotent when re-run', () async { + // A crash mid-ladder leaves user_version unchanged and re-runs every step + // from the top on a fresh connection, so each step is idempotent by + // contract. Re-running the function directly is the property that matters; + // a second open would not re-enter the rung at all, since the backfill is + // ladder-only by design. + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + await backfillDiveComputerGearTwins(db); + await backfillDiveComputerGearTwins(db); + + final count = await db + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + expect(count.read('c'), 2); + + final links = await db + .customSelect('SELECT COUNT(*) AS c FROM dive_equipment') + .getSingle(); + // dive1 -> c1, dive2 -> c1 and c2. + expect(links.read('c'), 3); + }); + + test( + 'adopts an unambiguous hand-created gear item instead of minting', + () async { + final native = NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute(''' + CREATE TABLE dive_computers ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + manufacturer TEXT, + model TEXT, + serial_number TEXT, + dive_count INTEGER NOT NULL DEFAULT 0, + is_favorite INTEGER NOT NULL DEFAULT 0, + notes TEXT NOT NULL DEFAULT '', + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute(''' + CREATE TABLE equipment ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + type TEXT NOT NULL, + brand TEXT, + model TEXT, + serial_number TEXT, + status TEXT NOT NULL DEFAULT 'active', + purchase_currency TEXT NOT NULL DEFAULT 'USD', + notes TEXT NOT NULL DEFAULT '', + is_active INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute( + "INSERT INTO dive_computers (id, diver_id, name, manufacturer, " + "model, created_at, updated_at) VALUES " + "('c1', 'd1', 'My Perdix', 'Shearwater', 'Perdix 2', 1, 1)", + ); + rawDb.execute( + "INSERT INTO equipment (id, diver_id, name, type, brand, model, " + "created_at, updated_at) VALUES " + "('hand-made', 'd1', 'Perdix', 'computer', 'Shearwater', " + "'Perdix 2', 1, 1)", + ); + }, + ); + final db = AppDatabase(native); + addTearDown(db.close); + + final row = await db + .customSelect( + "SELECT equipment_id FROM dive_computers WHERE id = 'c1'", + ) + .getSingle(); + expect(row.read('equipment_id'), 'hand-made'); + final count = await db + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + expect(count.read('c'), 1); + }, + ); +} From 41d5177ef764fb1b6536abb4c77a969e0531f042 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:28:40 -0400 Subject: [PATCH 09/19] feat(db): seed gear twins from the imported-computer self-heal --- .../database/imported_computer_backfill.dart | 50 ++++++++++++ .../imported_computer_gear_twin_test.dart | 77 +++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 test/core/database/imported_computer_gear_twin_test.dart diff --git a/lib/core/database/imported_computer_backfill.dart b/lib/core/database/imported_computer_backfill.dart index 2b88d8478c..c05ea9e8e7 100644 --- a/lib/core/database/imported_computer_backfill.dart +++ b/lib/core/database/imported_computer_backfill.dart @@ -1,5 +1,6 @@ import 'package:drift/drift.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; import 'package:submersion/core/database/imported_computer_identity.dart'; /// Register the dive computers that file-imported dives name, and attribute @@ -65,6 +66,20 @@ Future backfillImportedDiveComputers(DatabaseConnectionUser db) async { })) { return; } + // v169 gear twins: absent on an older fixture, in which case the mint + // below is skipped and the ladder seeds the twins instead. + final equipmentCols = await columnsOf('equipment'); + final hasGearColumn = + computerCols.contains('equipment_id') && + equipmentCols.containsAll({ + 'id', + 'diver_id', + 'name', + 'type', + 'created_at', + 'updated_at', + }); + final sourceCols = await columnsOf('dive_data_sources'); if (!sourceCols.containsAll({'dive_id', 'source_format'})) return; @@ -136,6 +151,41 @@ Future backfillImportedDiveComputers(DatabaseConnectionUser db) async { now, ], ); + + // Seed the gear twin, but ONLY when that insert actually inserted. If + // INSERT OR IGNORE no-opped because the computer already exists, the + // user may have deleted its gear item deliberately, and re-minting here + // would resurrect it on every app open. Same rule that keeps + // _backfillLegacyServiceSchedules out of beforeOpen. + if (hasGearColumn) { + final inserted = await db + .customSelect('SELECT changes() AS changed') + .getSingle(); + if (inserted.read('changed') > 0) { + final twinId = diveComputerGearId(computerId); + await db.customStatement( + 'INSERT OR IGNORE INTO equipment ' + '(id, diver_id, name, type, brand, model, serial_number, status, ' + 'purchase_currency, notes, is_active, created_at, updated_at) ' + "VALUES (?, ?, ?, 'computer', NULL, ?, ?, 'active', 'USD', '', 1, " + '?, ?)', + [ + twinId, + diverId, + trimmedModel, + trimmedModel, + (trimmedSerial?.isEmpty ?? true) ? null : trimmedSerial, + now, + now, + ], + ); + await db.customStatement( + 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', + [twinId, computerId], + ); + } + } + // Re-read so a later identity that normalizes onto this same device // adopts it instead of racing to insert it again. candidates = await _candidates(db); diff --git a/test/core/database/imported_computer_gear_twin_test.dart b/test/core/database/imported_computer_gear_twin_test.dart new file mode 100644 index 0000000000..4b3d2fd59d --- /dev/null +++ b/test/core/database/imported_computer_gear_twin_test.dart @@ -0,0 +1,77 @@ +import 'dart:io'; + +import 'package:drift/drift.dart' hide isNull; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart'; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; + +/// The #1288 self-heal registers computers named by file-imported dives with a +/// raw INSERT OR IGNORE, bypassing createComputer and therefore the gear-twin +/// hook there. It needs its own mint, and that mint must fire ONLY where the +/// computer row was genuinely inserted: otherwise a user who deleted the gear +/// item would get it back on the next app open. +void main() { + late Directory tempDir; + late File dbFile; + + setUp(() async { + // A file-backed database, because the heal runs in beforeOpen and this + // needs several opens of the same data. An in-memory database cannot be + // reopened and would lose its rows anyway. + tempDir = await Directory.systemTemp.createTemp('gear_twin_heal_'); + dbFile = File('${tempDir.path}/test.db'); + }); + tearDown(() async { + if (await tempDir.exists()) await tempDir.delete(recursive: true); + }); + + test('the heal mints a twin and does not re-mint a deleted one', () async { + final t = DateTime.now().millisecondsSinceEpoch; + + // Open once to create the schema, and seed a file-imported dive that names + // a computer with no registry row behind it. + final first = AppDatabase(NativeDatabase(dbFile)); + await first.customStatement( + 'INSERT INTO dives (id, dive_computer_model, dive_date_time, ' + 'created_at, updated_at) ' + "VALUES ('dive1', 'Perdix 2', 1, ?, ?)", + [t, t], + ); + await first.close(); + + // Reopen: beforeOpen runs the heal against that dive. + final second = AppDatabase(NativeDatabase(dbFile)); + final registered = await second + .customSelect('SELECT id, equipment_id FROM dive_computers') + .get(); + expect(registered, hasLength(1)); + final computerId = registered.single.read('id'); + final twinId = registered.single.read('equipment_id'); + expect(twinId, diveComputerGearId(computerId)); + + final gear = await second + .customSelect( + 'SELECT type FROM equipment WHERE id = ?', + variables: [Variable(twinId!)], + ) + .getSingle(); + expect(gear.read('type'), 'computer'); + + // The user deletes the gear item. The FK's setNull clears the link. + await second.customStatement('DELETE FROM equipment WHERE id = ?', [ + twinId, + ]); + await second.close(); + + // Reopen: the heal must NOT resurrect it. The computer row already exists, + // so INSERT OR IGNORE changes nothing and the mint is never reached. + final third = AppDatabase(NativeDatabase(dbFile)); + addTearDown(third.close); + final after = await third + .customSelect('SELECT COUNT(*) AS c FROM equipment') + .getSingle(); + expect(after.read('c'), 0); + }); +} From d6e4ee0fa90da1f6606cf014598550bcf0082386 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:29:27 -0400 Subject: [PATCH 10/19] fix(buoyancy): dive computers contribute no dry mass --- lib/core/buoyancy/gear_feature.dart | 8 ++++++ test/core/buoyancy/gear_feature_test.dart | 35 +++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/lib/core/buoyancy/gear_feature.dart b/lib/core/buoyancy/gear_feature.dart index 9af882788f..98a1d535b2 100644 --- a/lib/core/buoyancy/gear_feature.dart +++ b/lib/core/buoyancy/gear_feature.dart @@ -218,6 +218,9 @@ class GearFeature extends Equatable { EquipmentType.hood => 0.3, EquipmentType.gloves => 0.2, EquipmentType.boots => 0.4, + // Stated rather than left to the fallthrough, which already returns 0.0: + // gear twins (v169) make this a case readers will look for. + EquipmentType.computer => 0.0, _ => 0.0, }; @@ -225,6 +228,11 @@ class GearFeature extends Equatable { EquipmentType.wetsuit => 2.0, EquipmentType.drysuit => 3.0, EquipmentType.bcd => 3.5, + // A wrist computer's dry mass is negligible against the rig, and gear + // twins (v169) put one on every downloaded dive: the 0.5 kg fallthrough + // would move every diver's buoyancy by that much per computer. An explicit + // dry_weight_kg attribute still wins, so a bulky console can be modeled. + EquipmentType.computer => 0.0, _ => 0.5, }; diff --git a/test/core/buoyancy/gear_feature_test.dart b/test/core/buoyancy/gear_feature_test.dart index a9dba09688..8af44d647c 100644 --- a/test/core/buoyancy/gear_feature_test.dart +++ b/test/core/buoyancy/gear_feature_test.dart @@ -390,4 +390,39 @@ void main() { } }); }); + + group('dive computers', () { + test('contribute no dry mass', () { + // Gear twins (v169) put a computer on every downloaded dive. The + // _typeDryMass fallthrough of 0.5 kg would silently move every diver's + // rig by that much per computer. + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Perdix 2', + ); + expect(feature.dryMassKg, 0.0); + }); + + test('contribute no buoyancy prior', () { + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Perdix 2', + ); + expect(feature.priorKg, 0.0); + }); + + test('still honour an explicit user dry weight', () { + // A bulky console or canister is real mass; the attribute path stays + // live so a tech diver can still model it. + final feature = GearFeature.fromEquipment( + id: 'gear-1', + type: EquipmentType.computer, + name: 'Console', + weightKg: 1.2, + ); + expect(feature.dryMassKg, 1.2); + }); + }); } From a1b77dfa9542292f382656a45223817af40f4adb Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 19:31:18 -0400 Subject: [PATCH 11/19] feat(ui): show a dive computer's linked gear item on its detail page --- .../pages/device_detail_page.dart | 51 +++++++ lib/l10n/arb/app_ar.arb | 1 + lib/l10n/arb/app_de.arb | 1 + lib/l10n/arb/app_en.arb | 1 + lib/l10n/arb/app_es.arb | 1 + lib/l10n/arb/app_fr.arb | 1 + lib/l10n/arb/app_he.arb | 1 + lib/l10n/arb/app_hu.arb | 1 + lib/l10n/arb/app_it.arb | 1 + lib/l10n/arb/app_localizations.dart | 6 + lib/l10n/arb/app_localizations_ar.dart | 3 + lib/l10n/arb/app_localizations_de.dart | 3 + lib/l10n/arb/app_localizations_en.dart | 3 + lib/l10n/arb/app_localizations_es.dart | 3 + lib/l10n/arb/app_localizations_fr.dart | 3 + lib/l10n/arb/app_localizations_he.dart | 3 + lib/l10n/arb/app_localizations_hu.dart | 3 + lib/l10n/arb/app_localizations_it.dart | 3 + lib/l10n/arb/app_localizations_nl.dart | 3 + lib/l10n/arb/app_localizations_pt.dart | 3 + lib/l10n/arb/app_localizations_zh.dart | 3 + lib/l10n/arb/app_nl.arb | 1 + lib/l10n/arb/app_pt.arb | 1 + lib/l10n/arb/app_zh.arb | 1 + .../device_detail_page_gear_twin_test.dart | 124 ++++++++++++++++++ 25 files changed, 225 insertions(+) create mode 100644 test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart diff --git a/lib/features/dive_computer/presentation/pages/device_detail_page.dart b/lib/features/dive_computer/presentation/pages/device_detail_page.dart index b40c858c3f..89b38b6b98 100644 --- a/lib/features/dive_computer/presentation/pages/device_detail_page.dart +++ b/lib/features/dive_computer/presentation/pages/device_detail_page.dart @@ -9,6 +9,7 @@ import 'package:submersion/features/dive_computer/presentation/providers/reparse import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; import 'package:submersion/features/dive_log/presentation/providers/dive_computer_providers.dart'; import 'package:submersion/features/dive_log/presentation/providers/dive_providers.dart'; +import 'package:submersion/features/equipment/presentation/providers/equipment_providers.dart'; /// Page displaying details about a specific dive computer. class DeviceDetailPage extends ConsumerWidget { @@ -180,6 +181,8 @@ class DeviceDetailPage extends ConsumerWidget { context.l10n.diveComputer_detail_labelConnection, _getConnectionName(context, computer.connectionType), ), + if (computer.equipmentId != null) + _LinkedGearRow(equipmentId: computer.equipmentId!), ], ), ), @@ -655,3 +658,51 @@ class DeviceDetailPage extends ConsumerWidget { } } } + +/// The equipment row representing this device as gear, its gear twin (v169). +/// +/// Absent when the computer has no `equipmentId`, which is what deleting the +/// gear item leaves behind and is permanent by design: only a genuine +/// registration mints a twin. +class _LinkedGearRow extends ConsumerWidget { + const _LinkedGearRow({required this.equipmentId}); + + final String equipmentId; + + @override + Widget build(BuildContext context, WidgetRef ref) { + final theme = Theme.of(context); + final item = ref.watch(equipmentItemProvider(equipmentId)).valueOrNull; + if (item == null) return const SizedBox.shrink(); + + return InkWell( + onTap: () => context.push('/equipment/$equipmentId'), + child: Padding( + padding: const EdgeInsets.symmetric(vertical: 4), + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Text( + context.l10n.diveComputer_detail_linkedGear, + style: theme.textTheme.bodyMedium?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + ), + Row( + mainAxisSize: MainAxisSize.min, + children: [ + Text(item.name, style: theme.textTheme.bodyMedium), + const SizedBox(width: 4), + Icon( + Icons.chevron_right, + size: 18, + color: theme.colorScheme.onSurfaceVariant, + ), + ], + ), + ], + ), + ), + ); + } +} diff --git a/lib/l10n/arb/app_ar.arb b/lib/l10n/arb/app_ar.arb index aafdf5a549..cdb5b58c79 100644 --- a/lib/l10n/arb/app_ar.arb +++ b/lib/l10n/arb/app_ar.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "الطراز", "diveComputer_detail_labelName": "الاسم", "diveComputer_detail_lastDownload": "آخر تنزيل", + "diveComputer_detail_linkedGear": "قطعة المعدات", "diveComputer_detail_notesTitle": "الملاحظات", "diveComputer_detail_statisticsTitle": "الإحصائيات", "diveComputer_detail_unknown": "غير معروف", diff --git a/lib/l10n/arb/app_de.arb b/lib/l10n/arb/app_de.arb index ef7ac94779..aca9d56398 100644 --- a/lib/l10n/arb/app_de.arb +++ b/lib/l10n/arb/app_de.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modell", "diveComputer_detail_labelName": "Name", "diveComputer_detail_lastDownload": "Letzter Download", + "diveComputer_detail_linkedGear": "Ausrüstungsteil", "diveComputer_detail_notesTitle": "Notizen", "diveComputer_detail_statisticsTitle": "Statistiken", "diveComputer_detail_unknown": "Unbekannt", diff --git a/lib/l10n/arb/app_en.arb b/lib/l10n/arb/app_en.arb index 94edbcefdb..7e421643ac 100644 --- a/lib/l10n/arb/app_en.arb +++ b/lib/l10n/arb/app_en.arb @@ -12962,6 +12962,7 @@ "diveComputer_detail_labelModel": "Model", "diveComputer_detail_labelName": "Name", "diveComputer_detail_lastDownload": "Last Download", + "diveComputer_detail_linkedGear": "Gear item", "diveComputer_detail_notesTitle": "Notes", "diveComputer_detail_reimportAllButton": "Re-import all dives", "diveComputer_detail_reimportDialogBody": "Download every dive from {computerName} and review them against your log. This may take several minutes.", diff --git a/lib/l10n/arb/app_es.arb b/lib/l10n/arb/app_es.arb index 0ed0ead340..c2b125cefb 100644 --- a/lib/l10n/arb/app_es.arb +++ b/lib/l10n/arb/app_es.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modelo", "diveComputer_detail_labelName": "Nombre", "diveComputer_detail_lastDownload": "Ultima descarga", + "diveComputer_detail_linkedGear": "Equipo", "diveComputer_detail_notesTitle": "Notas", "diveComputer_detail_statisticsTitle": "Estadisticas", "diveComputer_detail_unknown": "Desconocido", diff --git a/lib/l10n/arb/app_fr.arb b/lib/l10n/arb/app_fr.arb index aadfc2e7f7..0169a7a085 100644 --- a/lib/l10n/arb/app_fr.arb +++ b/lib/l10n/arb/app_fr.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modele", "diveComputer_detail_labelName": "Nom", "diveComputer_detail_lastDownload": "Dernier telechargement", + "diveComputer_detail_linkedGear": "Équipement", "diveComputer_detail_notesTitle": "Notes", "diveComputer_detail_statisticsTitle": "Statistiques", "diveComputer_detail_unknown": "Inconnu", diff --git a/lib/l10n/arb/app_he.arb b/lib/l10n/arb/app_he.arb index b6a809dde0..e1c5aa7431 100644 --- a/lib/l10n/arb/app_he.arb +++ b/lib/l10n/arb/app_he.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "דגם", "diveComputer_detail_labelName": "שם", "diveComputer_detail_lastDownload": "הורדה אחרונה", + "diveComputer_detail_linkedGear": "פריט ציוד", "diveComputer_detail_notesTitle": "הערות", "diveComputer_detail_statisticsTitle": "סטטיסטיקה", "diveComputer_detail_unknown": "לא ידוע", diff --git a/lib/l10n/arb/app_hu.arb b/lib/l10n/arb/app_hu.arb index 65e7cec5c5..eebdbc9e70 100644 --- a/lib/l10n/arb/app_hu.arb +++ b/lib/l10n/arb/app_hu.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modell", "diveComputer_detail_labelName": "Nev", "diveComputer_detail_lastDownload": "Utolso letoltes", + "diveComputer_detail_linkedGear": "Felszerelés", "diveComputer_detail_notesTitle": "Megjegyzesek", "diveComputer_detail_statisticsTitle": "Statisztikak", "diveComputer_detail_unknown": "Ismeretlen", diff --git a/lib/l10n/arb/app_it.arb b/lib/l10n/arb/app_it.arb index 9db7ffe45f..680ac87a52 100644 --- a/lib/l10n/arb/app_it.arb +++ b/lib/l10n/arb/app_it.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modello", "diveComputer_detail_labelName": "Nome", "diveComputer_detail_lastDownload": "Ultimo download", + "diveComputer_detail_linkedGear": "Attrezzatura", "diveComputer_detail_notesTitle": "Note", "diveComputer_detail_statisticsTitle": "Statistiche", "diveComputer_detail_unknown": "Sconosciuto", diff --git a/lib/l10n/arb/app_localizations.dart b/lib/l10n/arb/app_localizations.dart index b1f6a64c45..2a529ce6d6 100644 --- a/lib/l10n/arb/app_localizations.dart +++ b/lib/l10n/arb/app_localizations.dart @@ -33852,6 +33852,12 @@ abstract class AppLocalizations { /// **'Last Download'** String get diveComputer_detail_lastDownload; + /// No description provided for @diveComputer_detail_linkedGear. + /// + /// In en, this message translates to: + /// **'Gear item'** + String get diveComputer_detail_linkedGear; + /// No description provided for @diveComputer_detail_notesTitle. /// /// In en, this message translates to: diff --git a/lib/l10n/arb/app_localizations_ar.dart b/lib/l10n/arb/app_localizations_ar.dart index 1b1842450d..5e681a2f7b 100644 --- a/lib/l10n/arb/app_localizations_ar.dart +++ b/lib/l10n/arb/app_localizations_ar.dart @@ -19922,6 +19922,9 @@ class AppLocalizationsAr extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'آخر تنزيل'; + @override + String get diveComputer_detail_linkedGear => 'قطعة المعدات'; + @override String get diveComputer_detail_notesTitle => 'الملاحظات'; diff --git a/lib/l10n/arb/app_localizations_de.dart b/lib/l10n/arb/app_localizations_de.dart index 3d2d698adc..a38c9daae8 100644 --- a/lib/l10n/arb/app_localizations_de.dart +++ b/lib/l10n/arb/app_localizations_de.dart @@ -20247,6 +20247,9 @@ class AppLocalizationsDe extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Letzter Download'; + @override + String get diveComputer_detail_linkedGear => 'Ausrüstungsteil'; + @override String get diveComputer_detail_notesTitle => 'Notizen'; diff --git a/lib/l10n/arb/app_localizations_en.dart b/lib/l10n/arb/app_localizations_en.dart index 0f134026aa..f4f4177fac 100644 --- a/lib/l10n/arb/app_localizations_en.dart +++ b/lib/l10n/arb/app_localizations_en.dart @@ -19943,6 +19943,9 @@ class AppLocalizationsEn extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Last Download'; + @override + String get diveComputer_detail_linkedGear => 'Gear item'; + @override String get diveComputer_detail_notesTitle => 'Notes'; diff --git a/lib/l10n/arb/app_localizations_es.dart b/lib/l10n/arb/app_localizations_es.dart index 5aafe0be76..25325172c4 100644 --- a/lib/l10n/arb/app_localizations_es.dart +++ b/lib/l10n/arb/app_localizations_es.dart @@ -20297,6 +20297,9 @@ class AppLocalizationsEs extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Ultima descarga'; + @override + String get diveComputer_detail_linkedGear => 'Equipo'; + @override String get diveComputer_detail_notesTitle => 'Notas'; diff --git a/lib/l10n/arb/app_localizations_fr.dart b/lib/l10n/arb/app_localizations_fr.dart index 3db988e35a..f7fff3e6c5 100644 --- a/lib/l10n/arb/app_localizations_fr.dart +++ b/lib/l10n/arb/app_localizations_fr.dart @@ -20355,6 +20355,9 @@ class AppLocalizationsFr extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Dernier telechargement'; + @override + String get diveComputer_detail_linkedGear => 'Équipement'; + @override String get diveComputer_detail_notesTitle => 'Notes'; diff --git a/lib/l10n/arb/app_localizations_he.dart b/lib/l10n/arb/app_localizations_he.dart index ccfa46f30d..984d84e987 100644 --- a/lib/l10n/arb/app_localizations_he.dart +++ b/lib/l10n/arb/app_localizations_he.dart @@ -19783,6 +19783,9 @@ class AppLocalizationsHe extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'הורדה אחרונה'; + @override + String get diveComputer_detail_linkedGear => 'פריט ציוד'; + @override String get diveComputer_detail_notesTitle => 'הערות'; diff --git a/lib/l10n/arb/app_localizations_hu.dart b/lib/l10n/arb/app_localizations_hu.dart index 2b68b6a99b..070c8018cf 100644 --- a/lib/l10n/arb/app_localizations_hu.dart +++ b/lib/l10n/arb/app_localizations_hu.dart @@ -20220,6 +20220,9 @@ class AppLocalizationsHu extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Utolso letoltes'; + @override + String get diveComputer_detail_linkedGear => 'Felszerelés'; + @override String get diveComputer_detail_notesTitle => 'Megjegyzesek'; diff --git a/lib/l10n/arb/app_localizations_it.dart b/lib/l10n/arb/app_localizations_it.dart index bc0d7a0f4d..0d68b0bff6 100644 --- a/lib/l10n/arb/app_localizations_it.dart +++ b/lib/l10n/arb/app_localizations_it.dart @@ -20282,6 +20282,9 @@ class AppLocalizationsIt extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Ultimo download'; + @override + String get diveComputer_detail_linkedGear => 'Attrezzatura'; + @override String get diveComputer_detail_notesTitle => 'Note'; diff --git a/lib/l10n/arb/app_localizations_nl.dart b/lib/l10n/arb/app_localizations_nl.dart index be697cd76f..71805956d9 100644 --- a/lib/l10n/arb/app_localizations_nl.dart +++ b/lib/l10n/arb/app_localizations_nl.dart @@ -20125,6 +20125,9 @@ class AppLocalizationsNl extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Laatste download'; + @override + String get diveComputer_detail_linkedGear => 'Uitrustingsstuk'; + @override String get diveComputer_detail_notesTitle => 'Notities'; diff --git a/lib/l10n/arb/app_localizations_pt.dart b/lib/l10n/arb/app_localizations_pt.dart index dc445d6034..feaa8c7d2b 100644 --- a/lib/l10n/arb/app_localizations_pt.dart +++ b/lib/l10n/arb/app_localizations_pt.dart @@ -20286,6 +20286,9 @@ class AppLocalizationsPt extends AppLocalizations { @override String get diveComputer_detail_lastDownload => 'Ultimo download'; + @override + String get diveComputer_detail_linkedGear => 'Equipamento'; + @override String get diveComputer_detail_notesTitle => 'Notas'; diff --git a/lib/l10n/arb/app_localizations_zh.dart b/lib/l10n/arb/app_localizations_zh.dart index 7e654ab690..7f7909ef8d 100644 --- a/lib/l10n/arb/app_localizations_zh.dart +++ b/lib/l10n/arb/app_localizations_zh.dart @@ -19262,6 +19262,9 @@ class AppLocalizationsZh extends AppLocalizations { @override String get diveComputer_detail_lastDownload => '上次下载'; + @override + String get diveComputer_detail_linkedGear => '装备'; + @override String get diveComputer_detail_notesTitle => '备注'; diff --git a/lib/l10n/arb/app_nl.arb b/lib/l10n/arb/app_nl.arb index 6793e9e744..aed2975f21 100644 --- a/lib/l10n/arb/app_nl.arb +++ b/lib/l10n/arb/app_nl.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Model", "diveComputer_detail_labelName": "Naam", "diveComputer_detail_lastDownload": "Laatste download", + "diveComputer_detail_linkedGear": "Uitrustingsstuk", "diveComputer_detail_notesTitle": "Notities", "diveComputer_detail_statisticsTitle": "Statistieken", "diveComputer_detail_unknown": "Onbekend", diff --git a/lib/l10n/arb/app_pt.arb b/lib/l10n/arb/app_pt.arb index b141d7c3de..34f5bd0bf1 100644 --- a/lib/l10n/arb/app_pt.arb +++ b/lib/l10n/arb/app_pt.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "Modelo", "diveComputer_detail_labelName": "Nome", "diveComputer_detail_lastDownload": "Ultimo download", + "diveComputer_detail_linkedGear": "Equipamento", "diveComputer_detail_notesTitle": "Notas", "diveComputer_detail_statisticsTitle": "Estatisticas", "diveComputer_detail_unknown": "Desconhecido", diff --git a/lib/l10n/arb/app_zh.arb b/lib/l10n/arb/app_zh.arb index 378c6c81c8..3dd44ebef9 100644 --- a/lib/l10n/arb/app_zh.arb +++ b/lib/l10n/arb/app_zh.arb @@ -6402,6 +6402,7 @@ "diveComputer_detail_labelModel": "型号", "diveComputer_detail_labelName": "名称", "diveComputer_detail_lastDownload": "上次下载", + "diveComputer_detail_linkedGear": "装备", "diveComputer_detail_notesTitle": "备注", "diveComputer_detail_statisticsTitle": "统计", "diveComputer_detail_unknown": "未知", diff --git a/test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart b/test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart new file mode 100644 index 0000000000..4222d7cc57 --- /dev/null +++ b/test/features/dive_computer/presentation/pages/device_detail_page_gear_twin_test.dart @@ -0,0 +1,124 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; + +import 'package:submersion/core/constants/enums.dart'; +import 'package:submersion/core/providers/provider.dart'; +import 'package:submersion/features/dive_computer/presentation/pages/device_detail_page.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; +import 'package:submersion/features/dive_log/presentation/providers/dive_computer_providers.dart'; +import 'package:submersion/features/equipment/domain/entities/equipment_item.dart'; +import 'package:submersion/features/equipment/presentation/providers/equipment_providers.dart'; +import 'package:submersion/features/settings/presentation/providers/settings_providers.dart'; +import 'package:submersion/l10n/arb/app_localizations.dart'; + +import '../../../../helpers/mock_providers.dart'; + +class _MockDiveComputerNotifier + extends StateNotifier>> + implements DiveComputerNotifier { + _MockDiveComputerNotifier() : super(const AsyncValue.data([])); + + @override + dynamic noSuchMethod(Invocation invocation) => null; +} + +DiveComputer _computer({String? equipmentId}) => DiveComputer( + id: 'comp-1', + name: 'My Perdix', + manufacturer: 'Shearwater', + model: 'Perdix 2', + equipmentId: equipmentId, + createdAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), +); + +EquipmentItem _gear() => const EquipmentItem( + id: 'gear-1', + name: 'Perdix 2 (wrist)', + type: EquipmentType.computer, +); + +Widget _buildTestWidget({required DiveComputer computer, EquipmentItem? gear}) { + final router = GoRouter( + initialLocation: '/dive-computers/comp-1', + routes: [ + GoRoute( + path: '/dive-computers/:id', + builder: (context, state) => + DeviceDetailPage(computerId: state.pathParameters['id']!), + ), + GoRoute( + path: '/equipment/:id', + builder: (context, state) => + const Scaffold(body: Text('EQUIPMENT_DETAIL_PAGE')), + ), + ], + ); + + return ProviderScope( + overrides: [ + settingsProvider.overrideWith((ref) => MockSettingsNotifier()), + diveComputerNotifierProvider.overrideWith( + (ref) => _MockDiveComputerNotifier(), + ), + diveComputerByIdProvider('comp-1').overrideWith((ref) async => computer), + equipmentItemProvider('gear-1').overrideWith((ref) async => gear), + ], + child: MaterialApp.router( + routerConfig: router, + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + locale: const Locale('en'), + ), + ); +} + +void main() { + testWidgets('shows the linked gear item', (tester) async { + await tester.pumpWidget( + _buildTestWidget( + computer: _computer(equipmentId: 'gear-1'), + gear: _gear(), + ), + ); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsOneWidget); + }); + + testWidgets('taps through to the equipment detail page', (tester) async { + await tester.pumpWidget( + _buildTestWidget( + computer: _computer(equipmentId: 'gear-1'), + gear: _gear(), + ), + ); + await tester.pumpAndSettle(); + + await tester.tap(find.text('Perdix 2 (wrist)')); + await tester.pumpAndSettle(); + + expect(find.text('EQUIPMENT_DETAIL_PAGE'), findsOneWidget); + }); + + testWidgets('shows no gear row when the twin was deleted', (tester) async { + // A null equipmentId is what deleting the gear item leaves behind, and it + // is permanent: nothing re-mints outside a genuine registration. + await tester.pumpWidget(_buildTestWidget(computer: _computer())); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsNothing); + }); + + testWidgets('shows no gear row when the equipment row is missing', ( + tester, + ) async { + await tester.pumpWidget( + _buildTestWidget(computer: _computer(equipmentId: 'gear-1'), gear: null), + ); + await tester.pumpAndSettle(); + + expect(find.text('Perdix 2 (wrist)'), findsNothing); + }); +} From 4719275b07bc4f74bf428216ffdc2f7545896aeb Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 21:10:38 -0400 Subject: [PATCH 12/19] fix: correct stray v168 references and cover the gear-twin error paths Addresses Copilot review on #1320. Three comments introduced while this branch still claimed v168 survived the renumber, because they live outside the six ladder places the renumber checklist covers: the gear-twin namespace doc, the GearTwinCandidate doc, and the parentRefs comment. A repo-wide sweep of the changed files confirms these were the only three; the remaining v168 in the migration test is correct, since that fixture upgrades FROM v168. Also drops the literal U+2014 from the plan's own no-em-dash constraint, so the document satisfies the rule it states. Adds two error-path tests. Both assert a documented guarantee that nothing covered: the resolver returns null rather than throwing when the write fails, because a computer that fails to get a twin is still a correctly registered computer and registration must not fail because gear seeding did; and the linker returns false rather than throwing, because linking must never abort a download that has already persisted the dive. --- .../plans/2026-08-26-dive-computer-gear-twin.md | 2 +- lib/core/database/dive_computer_gear_identity.dart | 4 ++-- lib/core/services/sync/sync_service.dart | 2 +- .../data/services/dive_computer_gear_linker_test.dart | 11 +++++++++++ .../services/dive_computer_gear_resolver_test.dart | 9 +++++++++ 5 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md index d74c996432..3937f2074a 100644 --- a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md +++ b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md @@ -14,7 +14,7 @@ - **Schema version is v169.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300), v167 (#1276) and v168 (#1237) are claimed by open PRs, and main is at v164. This plan originally claimed v168; #1237 renumbered onto it mid-implementation. Do NOT renumber without re-running that scan. - **`minimumCompatibleSchemaVersion` stays at 160.** The rule at `database.dart:3211` says not to raise it for a new nullable column. -- **Never use em-dashes (`—`, U+2014)** in any output: code, comments, docs, commit messages. En-dashes as prose punctuation and spaced hyphens are equally forbidden. Use commas, colons, semicolons, or two sentences. +- **Never use em-dashes (U+2014)** in any output: code, comments, docs, commit messages. En-dashes as prose punctuation and spaced hyphens are equally forbidden. Use commas, colons, semicolons, or two sentences. - **No emojis** in code, comments, or documentation. - **TDD.** Write the failing test first, watch it fail, then implement. - **Immutability.** Never mutate objects or arrays in place. diff --git a/lib/core/database/dive_computer_gear_identity.dart b/lib/core/database/dive_computer_gear_identity.dart index 336ac3f162..dbb99f5f35 100644 --- a/lib/core/database/dive_computer_gear_identity.dart +++ b/lib/core/database/dive_computer_gear_identity.dart @@ -2,7 +2,7 @@ import 'package:uuid/uuid.dart'; import 'package:submersion/core/database/imported_computer_identity.dart'; -/// Namespace for deterministic gear-twin ids (v168). +/// Namespace for deterministic gear-twin ids (v169). /// /// Frozen: every device must derive the same equipment id for the same /// registered computer, so changing this would fork one gear item into one per @@ -24,7 +24,7 @@ String diveComputerGearId(String computerId) => const Uuid().v5( /// An equipment row reduced to the fields the gear-twin match needs. /// /// Lets the rule live in one place: the repository builds these from Drift -/// rows, the v168 migration backfill from raw rows. +/// rows, the v169 migration backfill from raw rows. class GearTwinCandidate { const GearTwinCandidate({ required this.id, diff --git a/lib/core/services/sync/sync_service.dart b/lib/core/services/sync/sync_service.dart index a212a4339b..66e17c1fce 100644 --- a/lib/core/services/sync/sync_service.dart +++ b/lib/core/services/sync/sync_service.dart @@ -2057,7 +2057,7 @@ class SyncService { (field: 'computerId', parent: 'diveComputers', nullable: true), (field: 'sourceId', parent: 'diveDataSources', nullable: true), ], - // v168 gear twins: a peer's live computer whose gear item we deleted + // v169 gear twins: a peer's live computer whose gear item we deleted // locally would otherwise dangle this FK and abort the whole sync at // COMMIT. Nullable, so the computer survives with the reference cleared. 'diveComputers': [ diff --git a/test/features/equipment/data/services/dive_computer_gear_linker_test.dart b/test/features/equipment/data/services/dive_computer_gear_linker_test.dart index 5d7fd65bc0..c8fd584996 100644 --- a/test/features/equipment/data/services/dive_computer_gear_linker_test.dart +++ b/test/features/equipment/data/services/dive_computer_gear_linker_test.dart @@ -130,4 +130,15 @@ void main() { expect(await equipmentOn('dive1'), {'gear-1'}); }); + + test('returns false instead of throwing when the read fails', () async { + // Best-effort by contract: gear linking must never abort a download or + // import that has already persisted the dive. + await insertGear('gear-1'); + await insertComputer('c1', equipmentId: 'gear-1'); + await linkSource('dive1', 'c1'); + await db.customStatement('DROP TABLE dive_data_sources'); + + expect(await linker.linkComputerGearForDive(diveId: 'dive1'), isFalse); + }); } diff --git a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart index 21966dbb52..c891222004 100644 --- a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart +++ b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart @@ -167,4 +167,13 @@ void main() { expect(first, second); expect(await equipmentCount(), 1); }); + + test('returns null instead of throwing when the write fails', () async { + // Registration must not fail because gear seeding did: a computer with no + // twin is still a correctly registered computer. Dropping the table is the + // cheapest way to make every query in the resolver throw. + await db.customStatement('DROP TABLE equipment'); + + expect(await resolver.resolveGearTwin(computer()), isNull); + }); } From ee3ae2d089e7ee6801cc297d57bdc806319d0994 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 22:24:09 -0400 Subject: [PATCH 13/19] fix(db): declare the gear-twin FK on upgraded databases too Addresses the three suppressed findings in Copilot's second review on #1320. The first was a real behavioural bug, not a comment nit. _assertDiveComputerEquipmentColumn issued a bare `ALTER TABLE dive_computers ADD COLUMN equipment_id TEXT`. A fresh database picks up the FK from the table definition through onCreate, but an upgraded one gets only what the ALTER says, so `onDelete: setNull` held for new installs and not for existing users. Deleting a gear item would have left equipment_id pointing at a row that no longer exists, and the linker would then insert a dive_equipment row against a missing equipment id. The design calls that setNull load-bearing; on the population this feature exists for, it was absent. Now declared inline, matching the v158 _assertProfileSourceIdColumn precedent. The fix has a trap of its own, surfaced by migration_v66_test: SQLite accepts a REFERENCES clause naming a table that does not exist yet, then fails every later write to dive_computers with "no such table: main.equipment" once foreign keys are on. That test's fixture has no equipment table, and its DELETE FROM dive_computers began failing. The clause is therefore added only when equipment is present. Every real database has it, so production always takes the FK branch, and where the fallback applies there are no gear rows for the FK to act on. Also widens two PRAGMA guards to name every column their INSERT writes, rather than a subset that let the guard pass and the insert throw. The imported-computer one matters most: it runs unguarded inside beforeOpen, so a throw there fails app startup rather than degrading a feature. Adds a regression test asserting an UPGRADED database carries the FK, both by PRAGMA foreign_key_list and by the behaviour, deleting the gear row and checking the link clears. It failed before this change. Full suite: 20546 passed, 19 skipped. analyze clean. --- ...26-08-26-dive-computer-gear-twin-design.md | 17 +++++ lib/core/database/database.dart | 33 +++++++-- .../database/dive_computer_gear_backfill.dart | 6 ++ .../database/imported_computer_backfill.dart | 9 +++ ...igration_v169_dive_computer_gear_test.dart | 70 +++++++++++++++++++ 5 files changed, 130 insertions(+), 5 deletions(-) diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md index e5f8f73f71..c91b9d7ef0 100644 --- a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -196,6 +196,23 @@ points at nothing would hold a dangling reference. The `onDelete: setNull` is load-bearing. It is the mechanism, not a convenience: it implements half of the deletion semantics in D6. +**Correction, found in review.** Declaring it on the table class alone is not +enough. A fresh database gets the FK from that declaration through `onCreate`, +but an upgraded one gets whatever the migration's `ALTER TABLE` says, and a bare +`ADD COLUMN equipment_id TEXT` carries no constraint. That would have left +`setNull` true only for new installs while existing users, the population this +feature exists for, kept `equipment_id` pointing at deleted rows. The assert +helper must spell out `REFERENCES equipment(id) ON DELETE SET NULL`, matching +the v158 `_assertProfileSourceIdColumn` precedent. + +A second trap sits behind the first: SQLite accepts a reference to a table that +does not exist at `ALTER` time, then fails *every subsequent write* to +`dive_computers` with `no such table: main.equipment` once foreign keys are on. +The v66 migration test, whose fixture has no `equipment` table, is what surfaced +this. So the clause is added only when `equipment` is present. Every real +database has it, so production always takes the FK branch; where the bare +fallback applies there are no gear rows for the FK to act on anyway. + ### D2. Deterministic twin id ```dart diff --git a/lib/core/database/database.dart b/lib/core/database/database.dart index 105e7de568..7202678a2d 100644 --- a/lib/core/database/database.dart +++ b/lib/core/database/database.dart @@ -4256,17 +4256,40 @@ class AppDatabase extends _$AppDatabase { /// from both onUpgrade and the beforeOpen backstop. Nullable with no default, /// because a null means "this computer has no gear item", which is also what /// a user deleting the gear item leaves behind. + /// + /// The REFERENCES clause is not decoration. Without it an upgraded database + /// gets a bare TEXT column while a freshly created one gets the FK from the + /// table definition, so `onDelete: setNull` would hold only for new installs + /// and existing users would be left with `equipment_id` pointing at a deleted + /// row. SQLite permits a REFERENCES clause on ADD COLUMN precisely because + /// this column is nullable and defaults to NULL. Mirrors the v158 + /// `_assertProfileSourceIdColumn` precedent. + /// + /// It is added ONLY when `equipment` actually exists. SQLite accepts a + /// reference to a missing table at ALTER time and then fails every later + /// write to `dive_computers` with "no such table: main.equipment" once + /// foreign keys are on, which would break minimal fixtures and any database + /// caught mid-upgrade. Every real database has `equipment`, so production + /// always takes the FK branch; the bare fallback is harmless where it + /// applies, because a database with no `equipment` table has no gear rows + /// whose deletion the FK would need to cascade. Future _assertDiveComputerEquipmentColumn() async { final cols = await customSelect( "PRAGMA table_info('dive_computers')", ).get(); if (cols.isEmpty) return; final names = cols.map((c) => c.read('name')).toSet(); - if (!names.contains('equipment_id')) { - await customStatement( - 'ALTER TABLE dive_computers ADD COLUMN equipment_id TEXT', - ); - } + if (names.contains('equipment_id')) return; + + final equipmentCols = await customSelect( + "PRAGMA table_info('equipment')", + ).get(); + final reference = equipmentCols.isEmpty + ? '' + : ' REFERENCES equipment(id) ON DELETE SET NULL'; + await customStatement( + 'ALTER TABLE dive_computers ADD COLUMN equipment_id TEXT$reference', + ); } /// v164: media.manual_elapsed_seconds (issue #1090). Idempotent; safe to diff --git a/lib/core/database/dive_computer_gear_backfill.dart b/lib/core/database/dive_computer_gear_backfill.dart index fcf7b56ec3..fd4395f4ba 100644 --- a/lib/core/database/dive_computer_gear_backfill.dart +++ b/lib/core/database/dive_computer_gear_backfill.dart @@ -36,6 +36,9 @@ Future backfillDiveComputerGearTwins(DatabaseConnectionUser db) async { })) { return; } + // Every column the INSERT below writes, not a subset: a guard that passes + // and then throws on the insert is worse than no guard, because the caller + // reads it as proof the write is safe. final equipmentCols = await columnsOf('equipment'); if (!equipmentCols.containsAll({ 'id', @@ -45,6 +48,9 @@ Future backfillDiveComputerGearTwins(DatabaseConnectionUser db) async { 'brand', 'model', 'serial_number', + 'status', + 'purchase_currency', + 'notes', 'is_active', 'created_at', 'updated_at', diff --git a/lib/core/database/imported_computer_backfill.dart b/lib/core/database/imported_computer_backfill.dart index c05ea9e8e7..0ff564c3f7 100644 --- a/lib/core/database/imported_computer_backfill.dart +++ b/lib/core/database/imported_computer_backfill.dart @@ -69,6 +69,8 @@ Future backfillImportedDiveComputers(DatabaseConnectionUser db) async { // v169 gear twins: absent on an older fixture, in which case the mint // below is skipped and the ladder seeds the twins instead. final equipmentCols = await columnsOf('equipment'); + // Every column the mint below writes. This runs unguarded inside beforeOpen, + // so a throw here does not degrade a feature, it fails app startup. final hasGearColumn = computerCols.contains('equipment_id') && equipmentCols.containsAll({ @@ -76,6 +78,13 @@ Future backfillImportedDiveComputers(DatabaseConnectionUser db) async { 'diver_id', 'name', 'type', + 'brand', + 'model', + 'serial_number', + 'status', + 'purchase_currency', + 'notes', + 'is_active', 'created_at', 'updated_at', }); diff --git a/test/core/database/migration_v169_dive_computer_gear_test.dart b/test/core/database/migration_v169_dive_computer_gear_test.dart index 9a1bb6031f..d0f6edc4ef 100644 --- a/test/core/database/migration_v169_dive_computer_gear_test.dart +++ b/test/core/database/migration_v169_dive_computer_gear_test.dart @@ -102,4 +102,74 @@ void main() { final names = cols.map((c) => c.read('name')).toSet(); expect(names, contains('equipment_id')); }); + + test('an UPGRADED database carries the FK, not just a fresh one', () async { + // The bare `ALTER TABLE ... ADD COLUMN equipment_id TEXT` that an upgrade + // runs has no REFERENCES clause, so onDelete: setNull would exist only on + // databases created from scratch. That is backwards: existing users are + // the ones who upgrade. Without the FK, deleting a gear item leaves + // dive_computers.equipment_id pointing at a row that no longer exists, and + // the linker then tries to insert a dive_equipment row against a missing + // equipment id. + final db = AppDatabase( + NativeDatabase.memory( + setup: (rawDb) { + rawDb.execute('PRAGMA user_version = 168'); + rawDb.execute(_preV169DiveComputers); + rawDb.execute(''' + CREATE TABLE equipment ( + id TEXT NOT NULL PRIMARY KEY, + diver_id TEXT, + name TEXT NOT NULL, + type TEXT NOT NULL, + brand TEXT, + model TEXT, + serial_number TEXT, + status TEXT NOT NULL DEFAULT 'active', + purchase_currency TEXT NOT NULL DEFAULT 'USD', + notes TEXT NOT NULL DEFAULT '', + is_active INTEGER NOT NULL DEFAULT 1, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL + ) + '''); + rawDb.execute( + "INSERT INTO equipment (id, name, type, created_at, updated_at) " + "VALUES ('gear-1', 'gear-1', 'computer', 1, 1)", + ); + rawDb.execute( + "INSERT INTO dive_computers (id, name, created_at, updated_at) " + "VALUES ('c1', 'My Perdix', 1, 1)", + ); + }, + ), + ); + addTearDown(db.close); + + // The declared FK is what carries the setNull behaviour. + final fks = await db + .customSelect("PRAGMA foreign_key_list('dive_computers')") + .get(); + final toEquipment = fks.where( + (r) => r.read('table') == 'equipment', + ); + expect( + toEquipment, + isNotEmpty, + reason: 'upgraded dive_computers has no FK to equipment', + ); + expect(toEquipment.first.read('on_delete'), 'SET NULL'); + + // And the behaviour itself: deleting the gear item clears the link rather + // than stranding it. + await db.customStatement( + "UPDATE dive_computers SET equipment_id = 'gear-1' WHERE id = 'c1'", + ); + await db.customStatement("DELETE FROM equipment WHERE id = 'gear-1'"); + + final row = await db + .customSelect("SELECT equipment_id FROM dive_computers WHERE id = 'c1'") + .getSingle(); + expect(row.read('equipment_id'), isNull); + }); } From 52d31148f9888e6ec985ea7a55af839dfbb966d5 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Wed, 26 Aug 2026 22:55:07 -0400 Subject: [PATCH 14/19] docs: correct D9, the v169 backfill is local-only by design Addresses the sync finding in Copilot's third review on #1320. The finding is factually right and the documentation was wrong, but the fix is the docs, not the code. Verified: _exportEquipment filters on `hlc > hlcSince` for an incremental sync, and the backfill makes no sync calls at all, so its rows carry a null HLC and do not go out incrementally. D9 and the PR description both claimed they replicate. They do not. Keeping it local-only is correct rather than an oversight. Every input is already synced and the twin id is derived, so each device produces identical rows when its own ladder runs. That is the _backfillDiveComputerIds pattern. Marking pending would push one record per computer plus one per (dive, computer) pair from every device in the fleet, so peers agree on rows they each derive anyway, which is the fleet re-sync the house style exists to avoid. Both reasons the original D9 gave were wrong. A peer on the previous schema has no equipment_id column, so the field is dropped on apply rather than dangling; a peer at v169 has run its own ladder and derived the same twin. And a base export passes hlcSince == null, so an adopting device does receive the backfilled rows. One real limitation is now documented rather than claimed away: a dive downloaded by a not-yet-upgraded peer and synced to an already-migrated device is not linked on that device during the rollout window. It is a missing join row on one device, not divergence in the twin, and it resolves when anyone edits that dive's gear. Adds a test asserting the minted rows carry no HLC, so the choice cannot be reversed silently, and states the reasoning at the backfill itself, which is where its absence let the docs drift. --- ...26-08-26-dive-computer-gear-twin-design.md | 56 +++++++++++++------ .../database/dive_computer_gear_backfill.dart | 24 ++++++++ .../dive_computer_gear_backfill_test.dart | 26 ++++++++- 3 files changed, 87 insertions(+), 19 deletions(-) diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md index c91b9d7ef0..4f245f4eb9 100644 --- a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -387,24 +387,44 @@ schema-only. It adds the column if missing; it does not backfill. ### D9. Sync -Both the minted twin rows and the `dive_equipment` join rows are marked pending -and replicate. - -This deliberately departs from #1064's local-only, HLC-neutral heal. Two reasons -specific to this feature: - -1. A device can receive a `dive_computers` row by sync whose `equipmentId` - points at a twin it never minted. That is a dangling reference, not a missing - convenience. The twin row must travel. -2. Per F11, sync adopt bypasses the ladder. A dive downloaded by a peer still on - v164 and synced to an already-migrated device would otherwise never be - linked on that device, because the ladder has already run and the linker only - fires at local creation seams. Replicating closes the window. - -The cost is one small composite-key record per (dive, computer) pair, once, -comparable to a single bulk gear edit the app already supports. Convergence is -safe because the twin id is deterministic (D2) and `dive_equipment` has a -composite natural key, so two devices deriving the same rows upsert to one. +**Corrected in review. The original D9 said the opposite and was wrong on both +of its stated reasons.** + +The runtime paths mark pending and replicate: the resolver when it mints a twin, +`createComputer` when it stamps `equipment_id`, and `bulkAddEquipment` for every +link the linker adds. + +The **v169 backfill is local-only and HLC-neutral**, like `_backfillDiveComputerIds`. +It stamps no HLC and marks nothing pending, so its writes never go out on an +incremental sync. This is correct rather than an oversight: every input is +already synced (`dive_computers`, `dives`, `dive_data_sources`) and the twin id +is derived, so every device produces identical rows independently when its own +ladder runs. Marking them pending would push one record per computer plus one +per (dive, computer) pair from every device in the fleet, to make peers agree on +rows they will each derive anyway. That fleet-wide re-sync is precisely the cost +F12 cites as the reason the #1064 heal stayed local-only. + +The original reasoning claimed two things that do not hold: + +1. *"A peer can receive a `dive_computers` row whose `equipment_id` points at a + twin it never minted."* It cannot. A peer still on the previous schema has no + `equipment_id` column, so the field is dropped on apply; a peer at v169 has + run its own ladder and derived the same twin. +2. *"Sync adopt bypasses the ladder, so the rows must replicate."* A base or full + export passes `hlcSince == null` and therefore carries every row regardless of + HLC, so a device adopting the cloud base receives the backfilled twins. + +**Known limitation, accepted.** A dive downloaded by a peer still on the previous +schema and synced to an already-migrated device is not linked on that device: its +ladder has run, the runtime linker fires only at local creation seams, and the +peer's own later backfill is HLC-neutral so it does not push. That is a missing +join row on one device during the rollout window, not divergence in the twin +itself, and it resolves the moment anyone edits that dive's gear. Closing it +would require either a fleet-wide re-sync or a link step on the sync-apply path, +and neither is worth that cost. + +`dive_computer_gear_backfill_test.dart` asserts the HLC-neutrality directly, so +the choice cannot be silently reversed. ### D10. Buoyancy diff --git a/lib/core/database/dive_computer_gear_backfill.dart b/lib/core/database/dive_computer_gear_backfill.dart index fd4395f4ba..2e4d8e7d9f 100644 --- a/lib/core/database/dive_computer_gear_backfill.dart +++ b/lib/core/database/dive_computer_gear_backfill.dart @@ -14,6 +14,30 @@ import 'package:submersion/core/database/dive_computer_gear_identity.dart'; /// New rows land on a deterministic id ([diveComputerGearId]), so every device /// in a synced fleet derives the same primary key and they converge under sync /// upsert rather than duplicating. +/// +/// LOCAL-ONLY AND HLC-NEUTRAL, deliberately. Nothing here marks a record +/// pending or stamps an HLC, so these writes never go out on an incremental +/// sync. That is the `_backfillDiveComputerIds` pattern and it is correct here +/// for the same reason: every input is already synced (`dive_computers`, +/// `dives`, `dive_data_sources`) and the twin id is derived, so every device +/// produces identical rows independently when its own ladder runs. Marking +/// them pending would push one record per computer plus one per (dive, +/// computer) pair from every device in the fleet, to make peers agree on rows +/// they will each derive anyway. +/// +/// Two consequences worth knowing before "fixing" this: +/// +/// * A base/full export passes `hlcSince == null` and therefore DOES carry +/// these rows, so a device adopting the cloud base is not missing them. +/// * A dive downloaded by a peer still on the previous schema and synced to an +/// already-migrated device is not linked on that device: its ladder has run, +/// the runtime linker fires only at local creation seams, and the peer's own +/// later backfill is HLC-neutral so it does not push. That is a missing join +/// row on one device during the rollout window, not divergence in the twin +/// itself, and it resolves the moment anyone edits that dive's gear. +/// +/// The runtime paths are the opposite and mark pending as usual: the resolver +/// when it mints a twin, and `bulkAddEquipment` for every link the linker adds. Future backfillDiveComputerGearTwins(DatabaseConnectionUser db) async { // PRAGMA-guarded like every other backfill helper: the ladder runs against // minimal fixtures and against databases caught mid-upgrade. PRAGMA diff --git a/test/core/database/dive_computer_gear_backfill_test.dart b/test/core/database/dive_computer_gear_backfill_test.dart index 802bda0d34..6a82fed6d5 100644 --- a/test/core/database/dive_computer_gear_backfill_test.dart +++ b/test/core/database/dive_computer_gear_backfill_test.dart @@ -65,7 +65,8 @@ NativeDatabase _seeded() { notes TEXT NOT NULL DEFAULT '', is_active INTEGER NOT NULL DEFAULT 1, created_at INTEGER NOT NULL, - updated_at INTEGER NOT NULL + updated_at INTEGER NOT NULL, + hlc TEXT ) '''); @@ -242,4 +243,27 @@ void main() { expect(count.read('c'), 1); }, ); + test( + 'is local-only: minted rows carry no HLC and never go out incrementally', + () async { + // Deliberate, and load-bearing. Every input is already synced and the twin + // id is derived, so each device produces identical rows when its own ladder + // runs. Stamping an HLC here would push one record per computer plus one + // per (dive, computer) pair from every device in the fleet, to make peers + // agree on rows they each derive anyway. Incremental export filters + // equipment on `hlc > watermark`, so a null HLC is exactly what keeps these + // writes off the wire; a base export ignores the watermark and still + // carries them. + final db = AppDatabase(_seeded()); + addTearDown(db.close); + + final rows = await db + .customSelect("SELECT hlc FROM equipment WHERE type = 'computer'") + .get(); + expect(rows, hasLength(2)); + for (final row in rows) { + expect(row.read('hlc'), isNull); + } + }, + ); } From f5def7a8415f759e49cefd789150e81d7a9295cc Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 00:35:19 -0400 Subject: [PATCH 15/19] fix(equipment): seed the gear twin with insertOrIgnore, never an upsert Addresses the two suppressed findings in Copilot's fourth review on #1320. The resolver minted with insertOnConflictUpdate, which contradicts the feature's central promise. "Seed once, then owned by the user" means the resolver must never mutate a row that already holds the derived id, but on conflict an upsert rewrites name, brand, model and serial from the registry and wipes whatever the user set. Step 2 of the resolution order normally prevents reaching the mint with a row present, so this was latent rather than active. It is not safe to rely on: the check and the write are not atomic, sync applies equipment rows through its own upsert, and this database is opened by two isolates, so a peer's twin can land in between. There is no case where overwriting is wanted, so insertOrIgnore is both safer and a truer statement of intent. It also matches the v169 backfill, which already used INSERT OR IGNORE. The existing rename test asserted only that no duplicate row appeared. It now also asserts the adopted row keeps the user's brand and model, so the property insertOrIgnore protects is actually covered rather than implied. Also swaps the linker's raw `row.data['computer_id'] as String?` for the typed `row.read('computer_id')`, matching every other read in this feature's code. --- .../data/services/dive_computer_gear_linker.dart | 2 +- .../data/services/dive_computer_gear_resolver.dart | 10 +++++++++- .../services/dive_computer_gear_resolver_test.dart | 10 ++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/features/equipment/data/services/dive_computer_gear_linker.dart b/lib/features/equipment/data/services/dive_computer_gear_linker.dart index cb9d4159da..903376efb5 100644 --- a/lib/features/equipment/data/services/dive_computer_gear_linker.dart +++ b/lib/features/equipment/data/services/dive_computer_gear_linker.dart @@ -84,7 +84,7 @@ class DiveComputerGearLinker { ) .get(); return rows - .map((row) => row.data['computer_id'] as String?) + .map((row) => row.read('computer_id')) .whereType() .toList(); } diff --git a/lib/features/equipment/data/services/dive_computer_gear_resolver.dart b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart index 20c7d35f30..0afd2cbd08 100644 --- a/lib/features/equipment/data/services/dive_computer_gear_resolver.dart +++ b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart @@ -79,9 +79,17 @@ class DiveComputerGearResolver { if (match != null) return match.id; final now = DateTime.now().millisecondsSinceEpoch; + // insertOrIgnore, never upsert. This is a seed: if a row already holds + // the derived id, it is the twin and it belongs to the user. An upsert + // would rewrite their name, brand, model and serial from the registry. + // The step-2 check above normally prevents reaching here with a row + // present, but the two are not atomic: sync applies equipment rows + // concurrently, and this database is opened by two isolates, so a peer's + // twin can land between the check and this write. await _db .into(_db.equipment) - .insertOnConflictUpdate( + .insert( + mode: InsertMode.insertOrIgnore, EquipmentCompanion.insert( id: derivedId, diverId: Value(computer.diverId), diff --git a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart index c891222004..9488a898cf 100644 --- a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart +++ b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart @@ -122,6 +122,16 @@ void main() { expect(id, diveComputerGearId('c1')); expect(await equipmentCount(), 1); + + // Adopted, not rewritten. "Seed once, then owned by the user" means the + // resolver must never mutate a row that already holds the derived id, so + // the mint is insertOrIgnore rather than an upsert: an upsert would put + // the registry's Shearwater / Perdix 2 back over the user's own text. + final row = await (db.select( + db.equipment, + )..where((t) => t.id.equals(id!))).getSingle(); + expect(row.brand, 'Totally'); + expect(row.model, 'Renamed'); }); test('adopts an unambiguous hand-created gear item', () async { From 4c22cbc13286b13025b92f93459eb2eea422c0e9 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 08:52:08 -0400 Subject: [PATCH 16/19] fix(equipment): mark the twin pending only when the insert inserted Addresses the sync finding in Copilot's sixth review on #1320. resolveGearTwin called markRecordPending unconditionally after an insertOrIgnore. When the insert is ignored, because a peer's twin or another isolate landed in the race window the previous commit describes, that row is not our write. This is worse than wasted traffic. markRecordPending stamps an HLC on the entity row inside its transaction (sync_repository.dart:485, _stampHlc), so marking an adopted row bumps someone else's row to our local clock and queues it for export. Our unchanged copy can then win a later conflict comparison against a genuine edit made on the device that actually created the row. Now guarded on `SELECT changes()`, the same idiom imported_computer_backfill uses for exactly this question, which also makes the two seeding paths consistent. Two tests added: minting queues sync work, and adopting an existing row queues none. Note that neither exercises the ignored-insert branch itself: step 2 of the resolution order returns early whenever a row already holds the derived id, so the mint cannot be reached with one present except through the real race, which a unit test cannot stage. The guard is defensive for that window; the tests cover the two reachable paths. --- .../services/dive_computer_gear_resolver.dart | 23 ++++++++++---- .../dive_computer_gear_resolver_test.dart | 30 +++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lib/features/equipment/data/services/dive_computer_gear_resolver.dart b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart index 0afd2cbd08..2b94b8f142 100644 --- a/lib/features/equipment/data/services/dive_computer_gear_resolver.dart +++ b/lib/features/equipment/data/services/dive_computer_gear_resolver.dart @@ -102,11 +102,24 @@ class DiveComputerGearResolver { updatedAt: now, ), ); - await _syncRepository.markRecordPending( - entityType: 'equipment', - recordId: derivedId, - localUpdatedAt: now, - ); + + // Only mark pending when the insert actually inserted. If it was ignored + // because a peer's twin or another isolate landed in the race window + // above, this row is not our write: markRecordPending stamps an HLC on + // the entity row, so marking it would bump someone else's row to our + // clock and queue it for export, letting our unchanged copy win a later + // conflict against a genuine edit from the device that created it. + // Same `SELECT changes()` idiom the imported-computer heal uses. + final inserted = await _db + .customSelect('SELECT changes() AS changed') + .getSingle(); + if (inserted.read('changed') > 0) { + await _syncRepository.markRecordPending( + entityType: 'equipment', + recordId: derivedId, + localUpdatedAt: now, + ); + } return derivedId; } catch (e, stackTrace) { _log.error( diff --git a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart index 9488a898cf..8a3c2cae4a 100644 --- a/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart +++ b/test/features/equipment/data/services/dive_computer_gear_resolver_test.dart @@ -186,4 +186,34 @@ void main() { expect(await resolver.resolveGearTwin(computer()), isNull); }); + + Future pendingCountFor(String equipmentId) async { + final row = await db + .customSelect( + "SELECT COUNT(*) AS c FROM sync_records " + "WHERE entity_type = 'equipment' AND record_id = ?", + variables: [Variable(equipmentId)], + ) + .getSingle(); + return row.read('c'); + } + + test('marks the twin pending when it actually mints one', () async { + final id = await resolver.resolveGearTwin(computer()); + + expect(await pendingCountFor(id!), 1); + }); + + test('adopting an existing row queues no sync work', () async { + // Adoption is not a local edit. markRecordPending stamps an HLC on the + // entity row, so marking an adopted row would bump someone else's row to + // our clock and push it, which is how an unchanged copy wins a conflict it + // should have lost. + await insertGear('hand-made', brand: 'Shearwater', model: 'Perdix 2'); + + final id = await resolver.resolveGearTwin(computer()); + + expect(id, 'hand-made'); + expect(await pendingCountFor('hand-made'), 0); + }); } From a49b9e1e928746cf27cc3d9912a9c20fe1ecaffa Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 09:11:21 -0400 Subject: [PATCH 17/19] docs: follow the v169 -> v175 renumber into the design doc The merge of main into this branch on 2026-08-27 renumbered the schema claim from v169 to v175, after #1322 (which held v170) and others landed. The code side of that renumber was complete and correct: scalar, ladder entry, assert docstring, the if (from < 175) guard and its reportProgress twin, the beforeOpen backstop comment, and the migration test filename with its greaterThanOrEqualTo and contains assertions. Verified after the merge that the ladder is monotonic and unique and that the scalar equals its maximum, that lib/ and test/ carry no stale v169, and that the parentRefs entry and all four feature files survived. The docs had not followed. The design doc now says v175 throughout and records both renumbers, since a reader hitting v168 or v169 in the history should be able to tell which number shipped. The plan keeps its v169 snippets and gains a note at the top saying so. It is a record of the steps as executed rather than a description of current state, and rewriting twenty snippets that were accurate when written would make it less honest, not more. Full suite after the merge: 21186 passed, 19 skipped. analyze clean. --- .../2026-08-26-dive-computer-gear-twin.md | 6 ++++ ...26-08-26-dive-computer-gear-twin-design.md | 30 ++++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md index 3937f2074a..45f9aca1ea 100644 --- a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md +++ b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md @@ -10,6 +10,12 @@ **Spec:** `docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md` +> **Schema number, after the fact:** this plan was written against v169 and +> every snippet below says so. The claim ended up at **v175**, renumbered when +> main was merged into the branch on 2026-08-27 after #1322 and others landed. +> The snippets are left as written, since this document records the steps as +> they were executed; the shipped numbers live in the design doc and the code. + ## Global Constraints - **Schema version is v169.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300), v167 (#1276) and v168 (#1237) are claimed by open PRs, and main is at v164. This plan originally claimed v168; #1237 renumbered onto it mid-implementation. Do NOT renumber without re-running that scan. diff --git a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md index 4f245f4eb9..56133370f7 100644 --- a/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md +++ b/docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md @@ -4,7 +4,7 @@ **Issue:** none yet. Related to #1020 (equipment set keyed on the downloading computer), which this does not implement and does not block. **Branch:** `worktree-dive-computer-gear-twin` -**Schema:** claims v169. +**Schema:** claims v175. ## Problem @@ -137,12 +137,20 @@ awaits resolve in microtasks, and an unbroken microtask chain never reaches the vsync queue, freezing the migration spinner (`database.dart:3696-3726`, fixed with `if (processed++ % 25 == 24) await Future.delayed(Duration.zero);`). -**F13. v169 is the next free schema version.** Main is at v164 +**F13. The schema claim has moved twice; it is now v175.** Main is at v164 (`database.dart:3183`). A loop over open PR diffs returns v165 (#1290), v166 (#1300), v167 (#1276) and v168 (#1237); v138 (#603) is stale. Grepping main alone would have said v165 was free and walked into a silent auto-merge. -**Renumbered from v168 during implementation.** The first scan saw #1237 at +**Renumbered twice.** v168 -> v169 during implementation, then v169 -> v175 when +main was merged in on 2026-08-27 and #1322 (v170) and others had landed. The +scalar, the ladder entry, the assert docstring, the `if (from < N)` guard and its +`reportProgress` twin, the beforeOpen backstop comment, and the migration test +filename with its assertions all move together; verified after the second +renumber that the ladder is monotonic, unique, and that the scalar equals its +maximum. + +**Originally renumbered from v168 during implementation.** The first scan saw #1237 at v161. That PR was renumbered to v168 and pushed while this design was being written, so the claim was invisible to both the main grep and the open-PR scan at the moment they ran. Two branches writing the same scalar auto-merge with no @@ -307,7 +315,7 @@ samples. A file-imported dive registered by #1288 can have `computer_id` stamped and a data-source row while having no samples at all, and reusing the helper would have silently failed to link exactly the file-import case this feature was extended to cover. The linker owns a private query applying the same union the -v169 backfill uses, so the migration and the runtime path cannot disagree. +v175 backfill uses, so the migration and the runtime path cannot disagree. It performs no resolution and no minting. That is what makes deletion permanent: creation happens once at registration, linking happens per dive, and a cleared @@ -341,9 +349,9 @@ sets entirely. This ordering gets a regression test, not a comment. | Consolidation | covered for free, per F6 | | `importProfile` replaceSource branch (`isNewDive == false`) | new, per F7; idempotent through `insertOnConflictUpdate` | -### D8. Migration v169 +### D8. Migration v175 -Two passes in the `if (from < 169)` block, both PRAGMA-guarded like every +Two passes in the `if (from < 175)` block, both PRAGMA-guarded like every neighbouring helper. **Pass 1** resolves every existing `dive_computers` row through D3 and stamps @@ -375,14 +383,14 @@ pass 1 writes. The claim touches six places, all of which must move together if the number is renumbered before merge: the `currentSchemaVersion` scalar, the `migrationVersions` ladder entry, the `_assert*` helper docstring, the -`if (from < 169)` guard and its `reportProgress` twin, the `beforeOpen` backstop +`if (from < 175)` guard and its `reportProgress` twin, the `beforeOpen` backstop comment, and the migration test filename with its version assertions. The ladder is monotonic and unique but **not** contiguous by design (162 is permanently skipped, and 165 through 168 are reserved by open PRs); the audit must not "fix" that. A `beforeOpen` column assert is still required, per the F11 rule that a database -arriving already stamped at or above v169 enters no ladder block. That assert is +arriving already stamped at or above v175 enters no ladder block. That assert is schema-only. It adds the column if missing; it does not backfill. ### D9. Sync @@ -394,7 +402,7 @@ The runtime paths mark pending and replicate: the resolver when it mints a twin, `createComputer` when it stamps `equipment_id`, and `bulkAddEquipment` for every link the linker adds. -The **v169 backfill is local-only and HLC-neutral**, like `_backfillDiveComputerIds`. +The **v175 backfill is local-only and HLC-neutral**, like `_backfillDiveComputerIds`. It stamps no HLC and marks nothing pending, so its writes never go out on an incremental sync. This is correct rather than an oversight: every input is already synced (`dive_computers`, `dives`, `dive_data_sources`) and the twin id @@ -408,7 +416,7 @@ The original reasoning claimed two things that do not hold: 1. *"A peer can receive a `dive_computers` row whose `equipment_id` points at a twin it never minted."* It cannot. A peer still on the previous schema has no - `equipment_id` column, so the field is dropped on apply; a peer at v169 has + `equipment_id` column, so the field is dropped on apply; a peer at v175 has run its own ladder and derived the same twin. 2. *"Sync adopt bypasses the ladder, so the rows must replicate."* A base or full export passes `hlcSince == null` and therefore carries every row regardless of @@ -488,7 +496,7 @@ Tests first, per the project guide. * null-serial computers match on brand plus model * a computer whose twin was deleted resolves to nothing and does not re-mint -**Migration v169** +**Migration v175** * stranded-database fixture at v168 with computers, dives, and data sources * twins minted, `equipment_id` stamped, join rows inserted * a multi-source dive links **both** computers (the F2 case) From 72d9fd8f734593cb177715cae818700674edccd7 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 17:24:31 -0400 Subject: [PATCH 18/19] fix(dive-log): mark the registry row pending once, not twice Addresses Copilot's seventh review on #1320. createComputer marked the diveComputers row pending immediately after the insert and again after writing equipment_id. markRecordPending stamps a fresh HLC each time, so one logical creation spent two clock ticks. The mark now happens once, after the optional equipment_id write, so the row carries a single HLC representing its final state. It stays unconditional rather than moving inside the `if (twinId != null)` branch: a computer whose twin failed to resolve is still a registered computer and still has to sync. A test covers that, since the restructure is exactly the kind that quietly drops a guarantee. Note the double mark is not directly observable in sync_records, which keys on '_' and upserts, so both calls collapse to one row. The cost was the extra HLC tick, not a duplicate record. Also updates the two places in the plan that state the schema version as current fact rather than as a historical instruction: the Architecture summary and the Global Constraints line. The numbered task steps keep their v169 snippets, and the note at the top now says precisely which parts were corrected and which were deliberately left, rather than covering the whole document with one caveat. --- .../2026-08-26-dive-computer-gear-twin.md | 16 ++-- .../dive_computer_repository_impl.dart | 22 +++--- .../create_computer_gear_twin_test.dart | 79 +++++++++++++++++++ 3 files changed, 99 insertions(+), 18 deletions(-) create mode 100644 test/features/dive_log/data/repositories/create_computer_gear_twin_test.dart diff --git a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md index 45f9aca1ea..75c576032f 100644 --- a/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md +++ b/docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md @@ -4,21 +4,23 @@ **Goal:** A dive computer that downloaded a dive appears as a piece of equipment on that dive. -**Architecture:** A nullable `dive_computers.equipment_id` bridges the device registry to a real `equipment` row of type `computer` (its "gear twin"). The twin is created exactly once, at computer registration, at a deterministic UUID v5 id so a synced fleet converges on one row. A link-only service then attaches that twin to each dive at the non-interactive creation seams. A v169 migration backfills existing logbooks. +**Architecture:** A nullable `dive_computers.equipment_id` bridges the device registry to a real `equipment` row of type `computer` (its "gear twin"). The twin is created exactly once, at computer registration, at a deterministic UUID v5 id so a synced fleet converges on one row. A link-only service then attaches that twin to each dive at the non-interactive creation seams. A v175 migration backfills existing logbooks. **Tech Stack:** Flutter, Dart, Drift ORM, SQLite, Riverpod, `uuid` package. **Spec:** `docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md` -> **Schema number, after the fact:** this plan was written against v169 and -> every snippet below says so. The claim ended up at **v175**, renumbered when -> main was merged into the branch on 2026-08-27 after #1322 and others landed. -> The snippets are left as written, since this document records the steps as -> they were executed; the shipped numbers live in the design doc and the code. +> **Schema number, after the fact:** the shipped claim is **v175**. The Goal, +> Architecture and Global Constraints above state that, because they describe +> what was built. The numbered task steps below still say v169, and are left +> that way deliberately: they record the instructions as they were executed, +> and rewriting snippets that were accurate when written would misrepresent +> the history. The code and the design doc are the authority on the shipped +> number. ## Global Constraints -- **Schema version is v169.** Verified by diffing open PRs, not by grepping main: v165 (#1290), v166 (#1300), v167 (#1276) and v168 (#1237) are claimed by open PRs, and main is at v164. This plan originally claimed v168; #1237 renumbered onto it mid-implementation. Do NOT renumber without re-running that scan. +- **Schema version is v175.** It moved twice: v168 to v169 when #1237 was renumbered onto v168 mid-implementation, then v169 to v175 when main was merged in on 2026-08-27 after #1322 (v170) and others landed. Do NOT pick a number from the open-PR diff scan alone; it cannot see unpushed renumbers or local-only worktree claims, so scan every worktree's `currentSchemaVersion` as well. - **`minimumCompatibleSchemaVersion` stays at 160.** The rule at `database.dart:3211` says not to raise it for a new nullable column. - **Never use em-dashes (U+2014)** in any output: code, comments, docs, commit messages. En-dashes as prose punctuation and spaced hyphens are equally forbidden. Use commas, colons, semicolons, or two sentences. - **No emojis** in code, comments, or documentation. diff --git a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart index 9df7382d29..78091afcae 100644 --- a/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart +++ b/lib/features/dive_log/data/repositories/dive_computer_repository_impl.dart @@ -249,12 +249,6 @@ class DiveComputerRepository { ), ); - await _syncRepository.markRecordPending( - entityType: 'diveComputers', - recordId: id, - localUpdatedAt: now, - ); - // Seed the gear twin once, here, because this is the only repository // path that genuinely inserts a registry row (v175). Minting nowhere // else is what makes a user-deleted twin permanent. Pass the resolved @@ -267,13 +261,19 @@ class DiveComputerRepository { 'UPDATE dive_computers SET equipment_id = ? WHERE id = ?', [twinId, id], ); - await _syncRepository.markRecordPending( - entityType: 'diveComputers', - recordId: id, - localUpdatedAt: now, - ); } + // Marked pending ONCE, after the optional equipment_id write, so the row + // carries a single HLC representing its final state. Marking on either + // side of that update would spend two clock ticks on one logical + // creation. Unconditional: a computer whose twin failed to resolve is + // still a registered computer and still has to sync. + await _syncRepository.markRecordPending( + entityType: 'diveComputers', + recordId: id, + localUpdatedAt: now, + ); + // If a computer with this hardware identity was deleted earlier, its // dives kept provenance snapshots; give them their link back. await _relinkOrphanedRows(id, computer); diff --git a/test/features/dive_log/data/repositories/create_computer_gear_twin_test.dart b/test/features/dive_log/data/repositories/create_computer_gear_twin_test.dart new file mode 100644 index 0000000000..f2497e1ed4 --- /dev/null +++ b/test/features/dive_log/data/repositories/create_computer_gear_twin_test.dart @@ -0,0 +1,79 @@ +import 'package:drift/drift.dart' hide isNull; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:submersion/core/database/database.dart' hide DiveComputer; +import 'package:submersion/core/database/dive_computer_gear_identity.dart'; +import 'package:submersion/features/dive_log/data/repositories/dive_computer_repository_impl.dart'; +import 'package:submersion/features/dive_log/domain/entities/dive_computer.dart'; + +import '../../../../helpers/test_database.dart'; + +/// createComputer is the only repository path that genuinely inserts a registry +/// row, so it is where the gear twin is seeded. It marks the row pending ONCE, +/// after the optional equipment_id write, so the row carries a single HLC +/// representing its final state rather than two clock ticks for one creation. +void main() { + late AppDatabase db; + late DiveComputerRepository repo; + + setUp(() async { + db = await setUpTestDatabase(); + await db.customStatement('PRAGMA foreign_keys = OFF'); + repo = DiveComputerRepository(); + }); + tearDown(tearDownTestDatabase); + + DiveComputer computer({String id = 'c1'}) => DiveComputer( + id: id, + diverId: 'd1', + name: 'My Perdix', + manufacturer: 'Shearwater', + model: 'Perdix 2', + createdAt: DateTime.now(), + updatedAt: DateTime.now(), + ); + + Future pendingCountForComputer(String id) async { + final row = await db + .customSelect( + "SELECT COUNT(*) AS c FROM sync_records " + "WHERE entity_type = 'diveComputers' AND record_id = ?", + variables: [Variable(id)], + ) + .getSingle(); + return row.read('c'); + } + + test('registering a computer seeds and stores its gear twin', () async { + final created = await repo.createComputer(computer()); + + final expected = diveComputerGearId('c1'); + expect(created.equipmentId, expected); + + final row = await db + .customSelect("SELECT equipment_id FROM dive_computers WHERE id = 'c1'") + .getSingle(); + expect(row.read('equipment_id'), expected); + + final gear = await db + .customSelect( + 'SELECT type, name FROM equipment WHERE id = ?', + variables: [Variable(expected)], + ) + .getSingle(); + expect(gear.read('type'), 'computer'); + expect(gear.read('name'), 'My Perdix'); + }); + + test( + 'the registry row is still marked pending after the twin write', + () async { + // The pending mark moved to after the equipment_id update so the row gets + // one HLC instead of two. It must remain unconditional: a computer is a + // synced entity whether or not its twin resolved. + await repo.createComputer(computer()); + + expect(await pendingCountForComputer('c1'), 1); + }, + ); +} From 357dcce24c9332cbbbd883d8776428a22c19c700 Mon Sep 17 00:00:00 2001 From: Eric Griffin Date: Thu, 27 Aug 2026 18:16:13 -0400 Subject: [PATCH 19/19] test(db): bind parameters in the gear-twin backfill test queries Addresses the suppressed finding in Copilot's eighth review on #1320. Two helpers in dive_computer_gear_backfill_test interpolated values straight into SQL. The inputs are fixture constants so nothing was at risk here, but the project's Critical Rules say "Parameterized queries only" with no carve-out for tests, and a test is exactly where the pattern gets copied from. Both now bind through `variables: [Variable(...)]`. Swept every test file this PR touches rather than only the flagged line; these two were the only occurrences. --- test/core/database/dive_computer_gear_backfill_test.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/core/database/dive_computer_gear_backfill_test.dart b/test/core/database/dive_computer_gear_backfill_test.dart index 544882e114..189be6b890 100644 --- a/test/core/database/dive_computer_gear_backfill_test.dart +++ b/test/core/database/dive_computer_gear_backfill_test.dart @@ -1,3 +1,4 @@ +import 'package:drift/drift.dart' hide isNull; import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -108,7 +109,8 @@ NativeDatabase _seeded() { Future> _equipmentOn(AppDatabase db, String diveId) async { final rows = await db .customSelect( - "SELECT equipment_id FROM dive_equipment WHERE dive_id = '$diveId'", + 'SELECT equipment_id FROM dive_equipment WHERE dive_id = ?', + variables: [Variable(diveId)], ) .get(); return rows.map((r) => r.read('equipment_id')).toSet(); @@ -142,8 +144,8 @@ void main() { final row = await db .customSelect( - "SELECT name, type, brand, model FROM equipment " - "WHERE id = '${diveComputerGearId('c1')}'", + 'SELECT name, type, brand, model FROM equipment WHERE id = ?', + variables: [Variable(diveComputerGearId('c1'))], ) .getSingle(); expect(row.read('type'), 'computer');