-
Notifications
You must be signed in to change notification settings - Fork 34
Dive computers become equipment on the dives they logged (schema v175) #1320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
ac5841d
docs: design for dive computer gear twins
ericgriffin e475c3c
docs: implementation plan for dive computer gear twins
ericgriffin 11dc768
feat(equipment): deterministic gear-twin identity for dive computers
ericgriffin 27fa011
feat(db): add dive_computers.equipment_id gear-twin bridge at v169
ericgriffin 402fada
feat(equipment): seed a gear twin when a dive computer is registered
ericgriffin 7a7b64e
feat(equipment): attach dive computer gear twins at the import seams
ericgriffin 5a2bdc4
feat(equipment): link the gear twin when a dive source is replaced
ericgriffin 9e0773a
feat(db): backfill dive computer gear twins at v169
ericgriffin 41d5177
feat(db): seed gear twins from the imported-computer self-heal
ericgriffin d6e4ee0
fix(buoyancy): dive computers contribute no dry mass
ericgriffin a1b77df
feat(ui): show a dive computer's linked gear item on its detail page
ericgriffin 4719275
fix: correct stray v168 references and cover the gear-twin error paths
ericgriffin ee3ae2d
fix(db): declare the gear-twin FK on upgraded databases too
ericgriffin 52d3114
docs: correct D9, the v169 backfill is local-only by design
ericgriffin f5def7a
fix(equipment): seed the gear twin with insertOrIgnore, never an upsert
ericgriffin 26d3ae1
Merge branch 'main' into worktree-dive-computer-gear-twin
ericgriffin bcf528f
Merge branch 'main' into worktree-dive-computer-gear-twin
ericgriffin 4c22cbc
fix(equipment): mark the twin pending only when the insert inserted
ericgriffin f1c6722
Merge branch 'main' into worktree-dive-computer-gear-twin
ericgriffin a49b9e1
docs: follow the v169 -> v175 renumber into the design doc
ericgriffin 72d9fd8
fix(dive-log): mark the registry row pending once, not twice
ericgriffin 357dcce
test(db): bind parameters in the gear-twin backfill test queries
ericgriffin db7b155
merge: resolve the schema ladder with main, keeping both sides (v175)
ericgriffin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
2,380 changes: 2,380 additions & 0 deletions
2,380
docs/superpowers/plans/2026-08-26-dive-computer-gear-twin.md
Large diffs are not rendered by default.
Oops, something went wrong.
540 changes: 540 additions & 0 deletions
540
docs/superpowers/specs/2026-08-26-dive-computer-gear-twin-design.md
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| 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 (v175). | ||
| /// | ||
| /// 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. | ||
| /// | ||
| /// 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<void> 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<Set<String>> columnsOf(String table) async { | ||
| final rows = await db.customSelect("PRAGMA table_info('$table')").get(); | ||
| return rows.map((c) => c.read<String>('name')).toSet(); | ||
| } | ||
|
|
||
| final computerCols = await columnsOf('dive_computers'); | ||
| if (!computerCols.containsAll({ | ||
| 'id', | ||
| 'diver_id', | ||
| 'name', | ||
| 'manufacturer', | ||
| 'model', | ||
| 'serial_number', | ||
| 'equipment_id', | ||
| })) { | ||
| 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', | ||
| 'diver_id', | ||
| 'name', | ||
| 'type', | ||
| 'brand', | ||
| 'model', | ||
| 'serial_number', | ||
| 'status', | ||
| 'purchase_currency', | ||
| 'notes', | ||
| '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<String>('id'); | ||
| final diverId = computer.read<String?>('diver_id'); | ||
| final name = computer.read<String>('name'); | ||
| final manufacturer = computer.read<String?>('manufacturer'); | ||
| final model = computer.read<String?>('model'); | ||
| final serial = computer.read<String?>('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<String>(derivedId)], | ||
| ) | ||
| .getSingleOrNull(); | ||
|
|
||
| var twinId = byDerivedId?.read<String>('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<String>('id'), | ||
| diverId: r.read<String?>('diver_id'), | ||
| brand: r.read<String?>('brand'), | ||
| model: r.read<String?>('model'), | ||
| serialNumber: r.read<String?>('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 | ||
| '''); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 52d3114, by correcting the documentation rather than the code, since the finding is factually right but the behaviour is intentional.
Verified first:
_exportEquipmentfilters onhlc > hlcSincewhen incremental, and the backfill makes zero sync calls. So the rows do carry a null HLC and do not replicate incrementally. D9 and the PR description both claimed otherwise, and were wrong.Local-only is correct here. Every input is already synced and the twin id is a deterministic v5 derivation, so each device produces identical rows when its own ladder runs. That is the
_backfillDiveComputerIdspattern. Marking pending would push one record per computer plus one per (dive, computer) pair from every device, so peers agree on rows they each derive anyway.Both of D9's original arguments failed on checking: a peer on the old schema has no
equipment_idcolumn so nothing dangles, and a base export passeshlcSince == nullso adopting devices do receive the rows. One real limitation is now documented: 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. A test asserts the HLC-neutrality so it cannot be reversed silently.