diff --git a/docs/superpowers/plans/2026-07-06-dive-matching-time-gate-and-file-import-consolidation.md b/docs/superpowers/plans/2026-07-06-dive-matching-time-gate-and-file-import-consolidation.md new file mode 100644 index 000000000..bbb0c2255 --- /dev/null +++ b/docs/superpowers/plans/2026-07-06-dive-matching-time-gate-and-file-import-consolidation.md @@ -0,0 +1,593 @@ +# Dive matching time-gate + file-import consolidation 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:** Stop the file-import matcher from flagging dives that are far apart in time as duplicates, and let file imports manually Consolidate a matched dive (both prompted by ScubaBoard tester report, post 10791329). + +**Architecture:** Part 1 adds a necessary time-gate inside the pure `DiveMatcher` scoring function, fixing every file-import consumer at once. Part 2 ports the already-built-but-orphaned consolidation wiring (`performConsolidations` + `result.diveIdByIndex`) into the live `UniversalAdapter`, reusing `DiveConsolidationService` and the existing review UI (which reacts to the adapter's `supportedDuplicateActions`). + +**Tech Stack:** Flutter/Dart, Drift (SQLite), Riverpod, flutter_test + mockito, real-DB test harness (`setUpTestDatabase`). + +**Spec:** `docs/superpowers/specs/2026-07-06-dive-matching-time-gate-and-file-import-consolidation-design.md` + +## Global Constraints + +- All work happens in the worktree `/Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/dive-match-consolidate` (branch `worktree-dive-match-consolidate`). Paths below are repo-relative to that root. +- `dart format .` must produce no changes; `flutter analyze` must be clean across the whole project (not a filtered subset). +- No emojis in code, comments, or docs. Immutability; no object/array mutation of shared state. Proper error handling. +- TDD: write the failing test first, watch it fail, then implement. +- Commit messages: conventional style, imperative. Do NOT add `Co-Authored-By` trailers. +- Run codegen before tests (fresh worktree has no `*.g.dart` / `*.mocks.dart`). +- The 15-minute boundary is fixed (approved): a dive pair with `timeScore == 0.0` (starts >= 15 min apart) is never a match. + +## File Structure + +Production: +- `lib/features/dive_import/domain/services/dive_matcher.dart` — add the time-gate short-circuit in `calculateMatchScore`. (Part 1) +- `lib/features/universal_import/data/services/import_duplicate_checker.dart` — set `matchedExistingSource: true` on the Pass 0 `sourceUuid` match. (Part 2) +- `lib/features/import_wizard/data/adapters/universal_adapter.dart` — add `consolidate` to `supportedDuplicateActions`, treat `consolidate` like `importAsNew` in `_resolveSelections`, and fold consolidate-flagged dives in `performImport`. (Part 2) + +Tests: +- `test/features/dive_import/domain/services/dive_matcher_test.dart` — gate unit tests. (Part 1) +- `test/features/universal_import/data/services/import_duplicate_checker_test.dart` — `matchedExistingSource` + far-apart-not-matched. (Part 1 + 2) +- `test/features/import_wizard/data/adapters/universal_adapter_test.dart` — `supportedDuplicateActions` includes `consolidate`. (Part 2) +- `test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart` — NEW, real-DB fold. (Part 2) + +Reused unchanged: `performConsolidations` (`lib/features/universal_import/presentation/providers/import_consolidation_service.dart`) and its test — the adapter wraps its match map in the existing `ImportDuplicateResult(diveMatches: ...)`, so no signature change. + +--- + +### Task 0: Worktree bootstrap + +**Files:** none (environment only). + +- [ ] **Step 1: Initialize submodules** + +Run: `git -C /Users/ericgriffin/repos/submersion-app/submersion/.claude/worktrees/dive-match-consolidate submodule update --init --recursive` +Expected: submodules checked out (libdivecomputer etc.), no error. + +- [ ] **Step 2: Fetch packages** + +Run: `flutter pub get` +Expected: "Got dependencies!". + +- [ ] **Step 3: Run code generation** + +Run: `dart run build_runner build --delete-conflicting-outputs` +Expected: build completes; `lib/core/database/database.g.dart` and test `*.mocks.dart` exist. + +- [ ] **Step 4: Baseline the affected tests compile/pass** + +Run: `flutter test test/features/dive_import/domain/services/dive_matcher_test.dart test/features/universal_import/data/services/import_duplicate_checker_test.dart test/features/import_wizard/data/adapters/universal_adapter_test.dart test/features/universal_import/presentation/providers/import_consolidation_service_test.dart` +Expected: all PASS (green baseline before changes). + +--- + +### Task 1: Time-gate in `DiveMatcher` + +**Files:** +- Modify: `lib/features/dive_import/domain/services/dive_matcher.dart:24-34` +- Test: `test/features/dive_import/domain/services/dive_matcher_test.dart` + +**Interfaces:** +- Consumes: nothing new. +- Produces: no signature change. `DiveMatcher.calculateMatchScore(...)` now returns `0.0` whenever the two start times are >= 15 minutes apart, regardless of depth/duration. + +- [ ] **Step 1: Write the failing tests** + +Add these tests inside the existing `group('calculateMatchScore', ...)` block in `dive_matcher_test.dart` (after the "handles zero existing depth" test): + +```dart +test('gates to 0 for dives months apart even with identical ' + 'depth and duration (ScubaBoard report regression)', () { + // Months apart, both 47 min, similar depth previously scored exactly + // 0.50 = (0*0.5)+(1*0.3)+(1*0.2) and was flagged as a possible duplicate. + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 3, 15, 10, 0), + wearableMaxDepth: 18.0, + wearableDurationSeconds: 47 * 60, + existingStartTime: DateTime(2026, 6, 20, 14, 0), + existingMaxDepth: 18.0, + existingDurationSeconds: 47 * 60, + ); + + expect(score, 0.0); + expect(matcher.isPossibleDuplicate(score), isFalse); +}); + +test('gates to 0 for same-day dives hours apart with identical ' + 'depth and duration', () { + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 3, 15, 9, 0), + wearableMaxDepth: 18.0, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 3, 15, 14, 0), + existingMaxDepth: 18.0, + existingDurationSeconds: 45 * 60, + ); + + expect(score, 0.0); +}); + +test('gates to 0 exactly at the 15-minute boundary', () { + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 1, 15, 10, 0), + wearableMaxDepth: 18.5, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 1, 15, 10, 15), + existingMaxDepth: 18.5, + existingDurationSeconds: 45 * 60, + ); + + expect(score, 0.0); +}); + +test('still scores a near-in-time match within the 15-minute window', () { + // 14 min apart: timeScore = 1 - ((14-5)/10) = 0.1 + // composite = (0.1*0.5)+(1*0.3)+(1*0.2) = 0.55 -> still a possible match. + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 1, 15, 10, 0), + wearableMaxDepth: 18.5, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 1, 15, 10, 14), + existingMaxDepth: 18.5, + existingDurationSeconds: 45 * 60, + ); + + expect(score, greaterThan(0.0)); + expect(score, closeTo(0.55, 0.0001)); +}); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `flutter test test/features/dive_import/domain/services/dive_matcher_test.dart` +Expected: the four new tests FAIL (the months-apart / same-day / boundary tests currently return ~0.5 instead of 0.0). + +- [ ] **Step 3: Implement the gate** + +In `lib/features/dive_import/domain/services/dive_matcher.dart`, replace the body of `calculateMatchScore` (the block computing `timeScore`, `depthScore`, `durationScore`, and the weighted return) with: + +```dart +final timeScore = _calculateTimeScore(wearableStartTime, existingStartTime); + +// Time is a NECESSARY condition, not just a weighted term. `_calculateTimeScore` +// is 0.0 once the two starts are >= 15 min apart; with no time evidence, a +// depth + duration coincidence (0.30 + 0.20 = 0.50) must not be able to reach +// the possible-duplicate threshold. Two recordings that do not line up in time +// cannot be the same physical dive. +if (timeScore <= 0) return 0.0; + +final depthScore = _calculateDepthScore(wearableMaxDepth, existingMaxDepth); +final durationScore = _calculateDurationScore( + wearableDurationSeconds, + existingDurationSeconds, +); + +// Weighted composite score +return (timeScore * 0.50) + (depthScore * 0.30) + (durationScore * 0.20); +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `flutter test test/features/dive_import/domain/services/dive_matcher_test.dart` +Expected: PASS, including the pre-existing tests (identical -> >0.9; 8 min -> >0.7; 20 min -> `lessThanOrEqualTo(0.5)` still holds because 20 min now yields exactly 0.0; depth/duration/zero-depth cases have identical times so the gate does not trigger). + +- [ ] **Step 5: Commit** + +```bash +git add lib/features/dive_import/domain/services/dive_matcher.dart test/features/dive_import/domain/services/dive_matcher_test.dart +git commit -m "fix(import): gate dive matcher on time so far-apart dives are not flagged as duplicates" +``` + +--- + +### Task 2: Checker marks exact re-imports, and no longer matches far-apart dives + +**Files:** +- Modify: `lib/features/universal_import/data/services/import_duplicate_checker.dart:709-714` +- Test: `test/features/universal_import/data/services/import_duplicate_checker_test.dart` + +**Interfaces:** +- Consumes: `DiveMatcher` time-gate from Task 1 (far-apart pairs now score 0.0, so `_checkDiveDuplicates` Pass 1 does not record them). +- Produces: Pass 0 `sourceUuid` matches now carry `matchedExistingSource: true`; Pass 1 (content) matches keep `matchedExistingSource: false`. This is what makes the wizard default exact re-imports to skip and exclude them from bulk-consolidate. + +- [ ] **Step 1: Write the failing tests** + +In `import_duplicate_checker_test.dart`, inside `group('Dive duplicates (source_uuid)', ...)`, add a test asserting the flag (reuse the same payload/existing-dive construction the sibling "first-pass source_uuid match takes precedence" test at line 459 already uses; the incoming dive map carries `'sourceUuid': 'XYZ'` and `existingSourceUuidByDiveId` maps the existing dive's id to `'XYZ'`): + +```dart +test('source_uuid match flags matchedExistingSource so it defaults to ' + 'skip', () { + final result = const ImportDuplicateChecker().check( + payload: _payloadWithDive({ + 'dateTime': DateTime(2026, 1, 15, 10, 0), + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 45), + 'sourceUuid': 'XYZ', + }), + existingDives: [ + _existingDive( + id: 'existing-1', + dateTime: DateTime(2026, 1, 15, 10, 0), + maxDepth: 18.0, + runtime: const Duration(minutes: 45), + ), + ], + existingSites: const [], + existingTrips: const [], + existingEquipment: const [], + existingBuddies: const [], + existingDiveCenters: const [], + existingCertifications: const [], + existingTags: const [], + existingDiveTypes: const [], + existingSourceUuidByDiveId: const {'existing-1': 'XYZ'}, + ); + + final match = result.diveMatchFor(0); + expect(match, isNotNull); + expect(match!.matchedExistingSource, isTrue); +}); +``` + +In `group('Dive duplicates (fuzzy)', ...)`, add two tests — the content-match flag and the Part 1 far-apart regression at the checker level: + +```dart +test('content (fuzzy) match leaves matchedExistingSource false', () { + final result = const ImportDuplicateChecker().check( + payload: _payloadWithDive({ + 'dateTime': DateTime(2026, 1, 15, 10, 0), + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 45), + }), + existingDives: [ + _existingDive( + id: 'existing-1', + dateTime: DateTime(2026, 1, 15, 10, 2), + maxDepth: 18.0, + runtime: const Duration(minutes: 45), + ), + ], + existingSites: const [], + existingTrips: const [], + existingEquipment: const [], + existingBuddies: const [], + existingDiveCenters: const [], + existingCertifications: const [], + existingTags: const [], + existingDiveTypes: const [], + ); + + final match = result.diveMatchFor(0); + expect(match, isNotNull); + expect(match!.matchedExistingSource, isFalse); +}); + +test('does not match a dive far apart in time even with identical ' + 'depth and duration', () { + final result = const ImportDuplicateChecker().check( + payload: _payloadWithDive({ + 'dateTime': DateTime(2026, 3, 15, 10, 0), + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 47), + }), + existingDives: [ + _existingDive( + id: 'existing-1', + dateTime: DateTime(2026, 6, 20, 14, 0), + maxDepth: 18.0, + runtime: const Duration(minutes: 47), + ), + ], + existingSites: const [], + existingTrips: const [], + existingEquipment: const [], + existingBuddies: const [], + existingDiveCenters: const [], + existingCertifications: const [], + existingTags: const [], + existingDiveTypes: const [], + ); + + expect(result.diveMatches, isEmpty); +}); +``` + +Note: `_payloadWithDive` / `_existingDive` are the helper builders this test file already uses for its fuzzy and source_uuid groups. If their names differ, reuse whatever the neighboring tests in this file call to build a single-dive `ImportPayload` and an existing `Dive`. Do not invent new fixtures. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `flutter test test/features/universal_import/data/services/import_duplicate_checker_test.dart` +Expected: the `matchedExistingSource is true` test FAILS (currently the Pass 0 result has the default `false`). The other two should already pass after Task 1 (they document behavior); if the far-apart test fails, Task 1 was not applied. + +- [ ] **Step 3: Implement the flag** + +In `import_duplicate_checker.dart`, in `_checkDiveDuplicates`, the Pass 0 block that builds the `sourceUuid` match (currently constructs `DiveMatchResult(diveId: existing.id, score: _sourceUuidMatchScore, timeDifferenceMs: 0, siteName: existing.site?.name)`) — add `matchedExistingSource: true`: + +```dart +matches[i] = DiveMatchResult( + diveId: existing.id, + score: _sourceUuidMatchScore, + timeDifferenceMs: 0, + siteName: existing.site?.name, + matchedExistingSource: true, +); +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `flutter test test/features/universal_import/data/services/import_duplicate_checker_test.dart` +Expected: PASS (all three new tests plus the existing suite). + +- [ ] **Step 5: Commit** + +```bash +git add lib/features/universal_import/data/services/import_duplicate_checker.dart test/features/universal_import/data/services/import_duplicate_checker_test.dart +git commit -m "feat(import): flag exact source_uuid re-imports as existing-source so they default to skip" +``` + +--- + +### Task 3: `UniversalAdapter` offers and performs Consolidate for file imports + +**Files:** +- Modify: `lib/features/import_wizard/data/adapters/universal_adapter.dart` (imports; `supportedDuplicateActions` at line 140; `_resolveSelections` at line 699; `performImport` return block at lines 461-476) +- Test: `test/features/import_wizard/data/adapters/universal_adapter_test.dart` (supported-actions unit test) +- Create: `test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart` (real-DB fold) + +**Interfaces:** +- Consumes: `performConsolidations({required Set indices, required Map diveIdByIndex, required ImportDuplicateResult? duplicateResult, required DiveConsolidationService consolidationService, required DiveRepository diveRepository})` returning `ConsolidationSummary(consolidated, failed)` (unchanged); `diveConsolidationServiceProvider` (`Provider`); `UddfEntityImportResult` fields `dives` (int), `diveIds` (`List`), `diveIdByIndex` (`Map`); `ImportDuplicateResult(diveMatches: Map)`. +- Produces: `UniversalAdapter.supportedDuplicateActions` now includes `DuplicateAction.consolidate`; `performImport` returns a `UnifiedImportResult` whose `consolidatedCount` reflects folded dives, whose `importedCounts[dives]` excludes folded/failed dives, and whose `importedDiveIds` excludes tombstoned standalone dives. File imports never auto-consolidate (`currentComputerId` stays null). + +- [ ] **Step 1: Write the failing supported-actions unit test** + +In `universal_adapter_test.dart`, add (in whatever top-level `group` covers adapter metadata; if none, add a new `group('supportedDuplicateActions', ...)`): + +```dart +test('supports consolidate so the review UI offers it for file imports', () { + late UniversalAdapter adapter; + // Reuse this file's existing pattern for obtaining a WidgetRef-backed + // adapter (the buildBundle tests already construct one via a ProviderScope + // widget). Capture that adapter here. + // adapter = UniversalAdapter(ref: capturedRef); + expect( + adapter.supportedDuplicateActions, + containsAll({ + DuplicateAction.skip, + DuplicateAction.importAsNew, + DuplicateAction.consolidate, + }), + ); +}); +``` + +If constructing the adapter for a pure metadata check is awkward in this harness, instead assert against a directly-constructed adapter using a throwaway container ref, following the exact `UniversalAdapter(ref: ...)` construction the buildBundle tests already use in this file. + +- [ ] **Step 2: Run to verify it fails** + +Run: `flutter test test/features/import_wizard/data/adapters/universal_adapter_test.dart` +Expected: FAIL — `consolidate` is not yet in the set. + +- [ ] **Step 3: Add `consolidate` to supported actions and selection resolution** + +In `universal_adapter.dart`, change `supportedDuplicateActions` (line 140): + +```dart +@override +Set get supportedDuplicateActions => const { + DuplicateAction.skip, + DuplicateAction.importAsNew, + DuplicateAction.consolidate, +}; +``` + +And in `_resolveSelections` (line 715 loop), include consolidate-flagged indices in the import so they are persisted as standalone dives before folding: + +```dart +for (final entry in actions.entries) { + if (entry.value == DuplicateAction.importAsNew || + entry.value == DuplicateAction.consolidate) { + resolved.add(entry.key); + } +} +``` + +- [ ] **Step 4: Run to verify the unit test passes** + +Run: `flutter test test/features/import_wizard/data/adapters/universal_adapter_test.dart` +Expected: PASS (supported-actions test and the existing suite). + +- [ ] **Step 5: Write the failing real-DB integration test** + +Create `test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart`. Model the DB/service setup on `dive_computer_adapter_consolidate_integration_test.dart` (real `setUpTestDatabase()`, real `DiveRepository()`, real `DiveConsolidationService(diveRepository)`, seed a diver and a target dive), and model the adapter/provider wiring on `universal_adapter_test.dart`'s `performImport` harness (`_TestableImportNotifier.setPayload`, provider overrides, ProviderScope widget to obtain the adapter's `WidgetRef`). Override repos with REAL instances backed by the test DB (not mocks), and override `diveConsolidationServiceProvider` with the real `DiveConsolidationService`. + +Scenario and assertions: + +```dart +// Arrange: +// - diver 'diver-1' +// - existing target dive 'target-dive' at entryTime = DateTime.utc(2026,7,1,9), +// runtime 40 min, maxDepth 25.0, diverId 'diver-1' +// - a file-import ImportPayload with ONE dive overlapping the target: +// { 'dateTime': entryTime, 'maxDepth': 24.5, +// 'runtime': const Duration(minutes: 40), 'sourceUuid': 'sub-1' } +// (build it with the same payload helper the performImport tests use) +// - notifier.setPayload(payload) +// +// Act: +final checked = await adapter.checkDuplicates(await adapter.buildBundle()); +final result = await adapter.performImport( + checked, + {wizard.ImportEntityType.dives: {0}}, + {wizard.ImportEntityType.dives: {0: DuplicateAction.consolidate}}, +); + +// Assert: +expect(result.consolidatedCount, equals(1)); +// dives imported as NEW excludes the folded one: +expect(result.importedCounts[wizard.ImportEntityType.dives] ?? 0, equals(0)); +// Only the target dive remains; the standalone import was folded + tombstoned: +final allDives = await db.select(db.dives).get(); +expect(allDives, hasLength(1)); +expect(allDives.single.id, equals('target-dive')); +``` + +If `checkDuplicates` does not flag index 0 (e.g. because the seeded times must be within 15 min — they are identical here, so it will), fall back to constructing the `matchResults` directly on the bundle exactly like the download adapter integration test does (`EntityGroup(items: ..., duplicateIndices: {0}, matchResults: {0: DiveMatchResult(diveId: 'target-dive', score: 0.9, timeDifferenceMs: 0)})`), then call `performImport` with that bundle. + +- [ ] **Step 6: Run to verify it fails** + +Run: `flutter test test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart` +Expected: FAIL — `performImport` currently returns `consolidatedCount: 0` and leaves the imported dive standalone (2 dives in the DB). + +- [ ] **Step 7: Implement the fold in `performImport`** + +Add imports at the top of `universal_adapter.dart`: + +```dart +import 'package:submersion/features/dive_log/presentation/providers/dive_providers.dart' + show diveConsolidationServiceProvider; +import 'package:submersion/features/universal_import/data/services/import_duplicate_checker.dart' + show ImportDuplicateResult; +import 'package:submersion/features/universal_import/presentation/providers/import_consolidation_service.dart' + show performConsolidations; +``` + +(If `ImportDuplicateResult` or `DiveMatchResult` is already imported in this file, do not duplicate the import — merge the `show` clause instead.) + +Replace the final return of `performImport` (currently lines 461-476, from `final result = await importer.import(...)` through the `return UnifiedImportResult(...)`) with: + +```dart +final result = await importer.import( + data: uddfData, + selections: uddfSelections, + repositories: repos, + diverId: currentDiver.id, + retainSourceDiveNumbers: retainSourceDiveNumbers, + onProgress: onProgress, + cancelToken: cancelToken, +); + +// Fold consolidate-flagged dives (imported as standalone above) into their +// matched existing dive. File imports never auto-consolidate; these indices +// come only from an explicit user choice in the review step, and each such +// dive always has a match result (the UI offers Consolidate only on matches). +final diveActions = + duplicateActions[wizard.ImportEntityType.dives] ?? const {}; +final consolidateIndices = { + for (final entry in diveActions.entries) + if (entry.value == DuplicateAction.consolidate) entry.key, +}; + +var consolidated = 0; +var consolidationFailed = 0; +if (consolidateIndices.isNotEmpty) { + final matchResults = + bundle.groups[wizard.ImportEntityType.dives]?.matchResults ?? + const {}; + final summary = await performConsolidations( + indices: consolidateIndices, + diveIdByIndex: result.diveIdByIndex, + duplicateResult: ImportDuplicateResult(diveMatches: matchResults), + consolidationService: _ref.read(diveConsolidationServiceProvider), + diveRepository: repos.diveRepository, + ); + consolidated = summary.consolidated; + consolidationFailed = summary.failed; +} + +// `importer.import` counted folded (and failed-then-deleted) dives as +// imported; subtract them so the summary reports only genuinely NEW dives. +final counts = _convertImportCounts(result); +final netDives = result.dives - consolidated - consolidationFailed; +if (netDives > 0) { + counts[wizard.ImportEntityType.dives] = netDives; +} else { + counts.remove(wizard.ImportEntityType.dives); +} + +// Exclude tombstoned standalone dives from the imported-id list. +final removedDiveIds = { + for (final index in consolidateIndices) + if (result.diveIdByIndex[index] != null) result.diveIdByIndex[index]!, +}; +final netImportedDiveIds = [ + for (final id in result.diveIds) + if (!removedDiveIds.contains(id)) id, +]; + +return UnifiedImportResult( + importedCounts: counts, + consolidatedCount: consolidated, + skippedCount: skipped + consolidationFailed, + importedDiveIds: netImportedDiveIds, +); +``` + +- [ ] **Step 8: Run to verify the integration test passes** + +Run: `flutter test test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart` +Expected: PASS — `consolidatedCount == 1`, one dive in the DB, `importedCounts[dives]` is 0. + +- [ ] **Step 9: Run the adapter + checker suites together** + +Run: `flutter test test/features/import_wizard/data/adapters/ test/features/universal_import/` +Expected: PASS (no regressions in neighboring adapter/import tests). + +- [ ] **Step 10: Commit** + +```bash +git add lib/features/import_wizard/data/adapters/universal_adapter.dart test/features/import_wizard/data/adapters/universal_adapter_test.dart test/features/import_wizard/data/adapters/universal_adapter_consolidate_integration_test.dart +git commit -m "feat(import): support manual Consolidate for file imports in UniversalAdapter" +``` + +--- + +### Task 4: Whole-project verification and manual smoke + +**Files:** none (verification only). Also `git add` the spec + plan docs on the first commit here if not already committed. + +- [ ] **Step 1: Format** + +Run: `dart format .` +Expected: "0 changed" (or re-stage any files it reformats and amend the relevant commit). + +- [ ] **Step 2: Analyze (whole project)** + +Run: `flutter analyze` +Expected: "No issues found!". + +- [ ] **Step 3: Run the full affected-feature test set** + +Run: `flutter test test/features/dive_import/ test/features/universal_import/ test/features/import_wizard/` +Expected: all PASS. + +- [ ] **Step 4: Manual smoke (per CLAUDE.md — drive the real flow)** + +Run: `flutter run -d macos` +Then: import a Subsurface/UDDF file that contains (a) a dive already in the log from a different source and (b) an unrelated dive months from any existing dive. +Expected: (a) shows a possible-duplicate with a Consolidate option that folds into the existing dive; (b) is NOT flagged as a duplicate. Confirm the summary counts read correctly (imported vs consolidated). + +- [ ] **Step 5: Commit docs (if not yet committed)** + +```bash +git add docs/superpowers/specs/2026-07-06-dive-matching-time-gate-and-file-import-consolidation-design.md docs/superpowers/plans/2026-07-06-dive-matching-time-gate-and-file-import-consolidation.md +git commit -m "docs: spec and plan for dive matching time-gate and file-import consolidation" +``` + +--- + +## Self-Review + +**Spec coverage:** +- Part 1 time-gate → Task 1 (matcher) + Task 2 (checker regression). ✓ +- Part 2 offer consolidate → Task 3 Step 3. ✓ +- Part 2 `matchedExistingSource` defaults exact re-imports to skip → Task 2. ✓ +- Part 2 fold via `performConsolidations` → Task 3 Step 7. ✓ +- Part 2 manual-only (no auto-consolidate) → unchanged `currentComputerId == null`; noted in Task 3 Interfaces. ✓ +- Non-goals (no replaceSource, no matcher unification, no auto-consolidate) → honored; nothing in the plan adds them. ✓ +- Graceful same-computer/failed fold → covered by reused `performConsolidations` (already tested) + count-as-skipped in Task 3 Step 7. ✓ + +**Placeholder scan:** No "TBD"/"handle edge cases"/"similar to". The two test-helper references (`_payloadWithDive`/`_existingDive` in Task 2, harness reuse in Task 3) point at concrete existing patterns in the named files, with full arrange/act/assert given — not deferred work. + +**Type consistency:** `performConsolidations` called with `duplicateResult: ImportDuplicateResult(...)` matches its actual signature (verified against `import_consolidation_service.dart` and its test). `result.dives` / `result.diveIds` / `result.diveIdByIndex` match `UddfEntityImportResult`. `_resolveSelections`, `_convertImportCounts`, `_countSkipped` names match the current file. `DuplicateAction.consolidate`, `UnifiedImportResult` field names (`importedCounts`, `consolidatedCount`, `skippedCount`, `importedDiveIds`) match the download adapter's usage. diff --git a/docs/superpowers/specs/2026-07-06-dive-matching-time-gate-and-file-import-consolidation-design.md b/docs/superpowers/specs/2026-07-06-dive-matching-time-gate-and-file-import-consolidation-design.md new file mode 100644 index 000000000..79f52ee48 --- /dev/null +++ b/docs/superpowers/specs/2026-07-06-dive-matching-time-gate-and-file-import-consolidation-design.md @@ -0,0 +1,309 @@ +# Dive matching time-gate + file-import consolidation + +- **Date:** 2026-07-06 +- **Status:** Approved (design) +- **Scope:** Two related fixes in the import subsystem, prompted by a + ScubaBoard tester report (user *stiebs*, post 10791329). + +## Problem + +A tester imported a dive from Subsurface that Submersion flagged as a **50%** +"possible duplicate" of an existing dive, even though the two dives were: + +- months apart (different calendar dates), +- ~4.5 km apart (different sites), +- at different times of day, + +and shared **only** their duration (both 47 min). The tester also noted that +**Combine / Consolidate is only offered for direct dive-computer downloads, not +for file imports.** + +### Root cause 1 — the matcher lets depth + duration alone reach the threshold + +File imports (UDDF / Subsurface / universal) score candidate pairs with +`DiveMatcher.calculateMatchScore` +(`lib/features/dive_import/domain/services/dive_matcher.dart:17`), a weighted +sum: + +``` +score = timeScore*0.50 + depthScore*0.30 + durationScore*0.20 +``` + +- `timeScore` = 1.0 within 5 min, ramps to **0.0 at >= 15 min apart**. +- `depthScore` = 1.0 within 10%, 0.0 at >= 20% difference. +- `durationScore` = 1.0 within 3 min, 0.0 at >= 10 min difference. + +For the reported pair: `timeScore = 0.0` (months apart), `depthScore = 1.0` +(similar profile — common for recreational diving), `durationScore = 1.0` +(identical 47 min): + +``` +score = 0.00*0.50 + 1.00*0.30 + 1.00*0.20 = 0.50 +``` + +`isPossibleDuplicate` is `score >= 0.5` +(`dive_matcher.dart:96`), so the pair lands *exactly* on the threshold and is +flagged. **Time is treated as a compensable weighted term when it should be a +necessary gate:** a dive is a single event in time; two recordings that don't +line up in time cannot be the same dive, no matter how alike their depth and +duration. The dive-computer *download* path does not have this bug because it +pre-filters candidates with a SQL `WHERE ABS(time_diff) <= toleranceMinutes` +(default 5 min) before scoring — time is already a gate there. Only the +file-import matcher scores against every existing dive with no time gate. + +### Root cause 2 — the live file-import adapter never offers consolidation + +There are three import adapters. Only the download adapter supports +consolidation: + +| Path | `supportedDuplicateActions` | Calls `DiveConsolidationService`? | Routed? | +|---|---|---|---| +| `DiveComputerAdapter` (downloads) | skip, importAsNew, consolidate, replaceSource | yes | yes | +| `UniversalAdapter` (file imports) | **skip, importAsNew only** | **no** (`consolidatedCount: 0` hardcoded) | **yes** | +| `UniversalImportNotifier` + `ImportReviewStep` | consolidate supported | yes (`performConsolidations`) | **no (orphaned)** | + +The routed file-import adapter (`universal_adapter.dart:140`) simply omits +`consolidate` from its supported actions, and its `performImport` +(`universal_adapter.dart:461-476`) calls `UddfEntityImporter.import` and returns +a hardcoded `consolidatedCount: 0`. A complete, working consolidation path for +file imports (`import_consolidation_service.dart#performConsolidations` + +`UniversalImportNotifier.performImport`) already exists but is not mounted in the +router — the router points file imports at the unified wizard +(`UniversalAdapter`). + +Investigation **ruled out** a deeper blocker: `DiveConsolidationBuilder.classify` +(`lib/features/dive_log/domain/services/dive_consolidation_builder.dart:85`) does +**not** require a dive-computer serial. Its `sameComputer` guard only rejects a +*duplicate non-empty* serial; null/empty serials pass. File-imported dives +(which have `computerId == null` and often a null serial) consolidate fine. This +is a UI/wiring gap, not a model limitation. + +## Goals + +1. Stop the matcher from flagging dives that are far apart in time as possible + duplicates, without weakening detection of genuine duplicates. +2. Offer **manual** Combine / Consolidate for file imports, reusing the existing + `DiveConsolidationService` and the review UI the download flow already uses. + +## Non-goals + +- No change to the dive-computer download matcher + (`findMatchingDiveWithScore`) — it already gates on time. +- No unification of the two matchers' differing weight sets (50/30/20 vs + 40/35/25) or depth models (% vs absolute). Latent inconsistency, out of scope. +- No `replaceSource` for file imports (download-only for now). +- No **auto**-consolidation for file imports (see Part 2, decision 4). +- The other items in the tester's post (FIT multi-select, Windows photo + "file-not-found") are separate subsystems and out of scope. + +## Part 1 — Make time a necessary gate in `DiveMatcher` + +**Change:** in `calculateMatchScore`, short-circuit to "no match" when the time +evidence is already zero, before the weighted sum. + +```dart +final timeScore = _calculateTimeScore(wearableStartTime, existingStartTime); +// Time is a NECESSARY condition, not just a weighted term. `_calculateTimeScore` +// is already 0.0 once the starts are >= 15 min apart; at that point there is no +// time evidence the two recordings are the same physical dive, so a depth+ +// duration coincidence must not be able to reach the possible-duplicate +// threshold (depth 0.30 + duration 0.20 = 0.50). +if (timeScore <= 0) return 0.0; + +final depthScore = _calculateDepthScore(wearableMaxDepth, existingMaxDepth); +final durationScore = _calculateDurationScore( + wearableDurationSeconds, existingDurationSeconds, +); +return (timeScore * 0.50) + (depthScore * 0.30) + (durationScore * 0.20); +``` + +**Why the 15-min boundary is safe (low false-negative risk):** + +- A re-import of an old dive carries the dive's *original* timestamp, which + matches the existing entry to the second — the import date is irrelevant. +- Two computers on one diver enter the water within seconds; their entry times + are far inside 15 min. +- Cross-source timezone skew was already addressed + (see `2026-03-17-dive-time-timezone-fix-design.md`), so the two sides compare + on the same wall-clock basis (dive times are wall-clock-as-UTC by design). + +**Effect on the report:** the reported pair scores `0.0` instead of `0.50` and +is not flagged. Same-day-but-hours-apart collisions (morning vs afternoon dive +with matching depth+duration) are also fixed, since they too exceed 15 min. + +**Placement:** the gate lives in `calculateMatchScore`, so it fixes every +consumer at once — `UddfDuplicateChecker` and `ImportDuplicateChecker` both call +it. No checker-level *production* change is required for Part 1 (a checker-level +regression test is still added — see Testing plan). + +## Part 2 — Wire manual consolidation into `UniversalAdapter` + +Port the proven wiring from the orphaned +`UniversalImportNotifier.performImport` into the live adapter. Four edits: + +### 1. Offer the action + +`UniversalAdapter.supportedDuplicateActions` gains `DuplicateAction.consolidate`: + +```dart +Set get supportedDuplicateActions => const { + DuplicateAction.skip, + DuplicateAction.importAsNew, + DuplicateAction.consolidate, +}; +``` + +Because every layer of the review UI reads this one set +(`review_step.dart:40`, `entity_review_list.dart:994`, +`dive_comparison_card.dart:374`), the "Consolidate" button (per-card and bulk) +appears automatically — no widget changes. + +### 2. Mark exact re-imports so they default to skip + +`ImportDuplicateChecker._checkDiveDuplicates` +(`import_duplicate_checker.dart:670`) currently builds every `DiveMatchResult` +with `matchedExistingSource` defaulting to `false`. Set it **`true` in the Pass 0 +`sourceUuid` exact-match branch** (`import_duplicate_checker.dart:709`), leaving +Pass 1 (content-fuzzy) matches `false`. + +Consequences, all already implemented generically in the wizard providers: + +- `setBundle` forces `matchedExistingSource` matches to **skip** — an exact + re-import is never auto- or bulk-consolidated. +- `applyBulkAction`'s consolidate gate (`score >= 0.7 && !matchedExistingSource`) + excludes them from "Consolidate all". + +`matchedComputerId` is **not** populated for file imports: it is only consumed by +the auto-consolidate branch, which cannot fire for file imports (decision 4), so +adding it would be dead data and would force `_checkDiveDuplicates` async for no +benefit. + +### 3. Fold consolidate-flagged dives after import + +In `UniversalAdapter.performImport`: + +1. Read the consolidate-flagged dive indices from + `duplicateActions[ImportEntityType.dives]` (entries whose action is + `consolidate`). +2. Ensure those indices are included in the import selection (they must be + imported as standalone dives first, exactly like `importAsNew`, so their + cross-references resolve). Extend the existing `resolve()` / + `_resolveSelections` so `consolidate` is treated like `importAsNew` for + selection purposes. +3. After `importer.import(...)`, if any consolidate indices exist, call + `performConsolidations` with `result.diveIdByIndex`, the per-index match + results from `bundle.groups[dives].matchResults`, the + `diveConsolidationServiceProvider`, and the dive repository. +4. Return `consolidatedCount: summary.consolidated` (no longer hardcoded `0`), + and surface `summary.failed` (> 0) to the user (reuse the orphaned path's + warning wording: "N dive(s) could not be consolidated ... the partial imports + were removed again to avoid duplicates."). + +`performConsolidations` already imports-then-folds each dive and, on a fold +failure, deletes the freshly-imported standalone dive +(`bulkDeleteDives`, tombstone-honoring) so no bare duplicate is stranded. + +**Small refactor:** `performConsolidations` currently takes +`duplicateResult: ImportDuplicateResult?` and calls `diveMatchFor(index)`. The +adapter has a `Map` instead. Generalize the parameter to a +plain `Map matchByIndex` (or a +`DiveMatchResult? Function(int)` callback) and update its single existing caller +(`universal_import_providers.dart:638`). + +### 4. Manual only — no auto-consolidation for file imports + +`setBundle`'s auto-consolidate branch requires `currentComputerId != null` +(`import_wizard_providers.dart:288`). `UniversalAdapter.buildBundle` leaves +`currentComputerId` null (a file may contain dives from several computers, so +there is no single "current computer" and no confident cross-computer proof). +We keep it null: file-import duplicates stay pending and the user explicitly +chooses Consolidate. This is intentional and requires no code change. + +## Data flow (Part 2, happy path) + +``` +file import → checkDuplicates + Pass 0 sourceUuid exact → DiveMatchResult(matchedExistingSource: true) → default skip + Pass 1 content + TIME GATE→ DiveMatchResult(score>=0.5, existingSource:false) → pending +review UI shows Consolidate (per-card / bulk) because adapter now supports it +user picks Consolidate on a Pass-1 match +performImport: + import consolidate-flagged dives as standalone (importer.import → diveIdByIndex) + performConsolidations: apply(target = match.diveId, secondary = new dive) then tombstone + summary.consolidated → UnifiedImportResult.consolidatedCount +``` + +## Edge cases & risks + +- **Same physical computer, cross-source (Subsurface serial == an existing + download's serial), overlapping in time.** Pass 1 matches (source UUIDs + differ, so `matchedExistingSource` is false) and Consolidate is offered. On + apply, `DiveConsolidationBuilder.classify` returns `sameComputer` and throws; + `performConsolidations` catches it, deletes the just-imported standalone dive, + and reports it under `summary.failed` → the user sees a non-fatal warning. No + crash, no stranded duplicate. Suppressing the *offer* for equal-serial matches + is a possible future refinement but is **deferred** — the graceful-failure path + already prevents harm, and this scenario is uncommon. +- **Import + fold are not one transaction** (pre-existing property of + `performConsolidations`). Compensating delete keeps the DB consistent on + partial failure. +- **Time gate false negative.** Only if two recordings of the same dive differ + by >= 15 min in start time — precluded by the reasons in Part 1. Accepted. +- **Orphaned code.** `UniversalImportNotifier.performImport` / + `ImportReviewStep` remain in the tree, still unrouted. This change does not + depend on them; leave them as-is (a separate cleanup may remove them later). + +## Testing plan (TDD — tests first) + +**Part 1 — `dive_matcher_test.dart`:** +- Months apart + identical duration + depth within 10% → `score == 0.0`, not + `isPossibleDuplicate` (regression test for the exact report). +- ~4 hours apart, same depth+duration → `0.0` (same-day collision). +- Just over 15 min apart → gated to `0.0`; within 15 min → weighted score as + before. +- Existing "real duplicate" cases (identical, 8-min offset, etc.) still score + as before — the gate must not regress them. +- Checker-level: add a `ImportDuplicateChecker` / `UddfDuplicateChecker` case + where a far-apart existing dive is **not** returned as a match. + +**Part 2:** +- `ImportDuplicateChecker`: Pass 0 sourceUuid match sets + `matchedExistingSource: true`; Pass 1 content match leaves it `false`. +- `UniversalAdapter.supportedDuplicateActions` includes `consolidate`. +- `UniversalAdapter.performImport`: a consolidate-flagged dive is imported then + folded via `DiveConsolidationService.apply`, `consolidatedCount == 1`, and the + standalone dive is tombstoned. (Mirror + `dive_computer_adapter_consolidate_integration_test.dart`.) +- Same-serial target → fold rejected, standalone deleted, reported as failed, + import otherwise succeeds. +- `performConsolidations` signature change: update its existing test/caller. + +**Whole-project gates (per CLAUDE.md):** `dart format .`, `flutter analyze`, +`flutter test` on affected files, then drive an actual file import to confirm the +Consolidate button appears and folds. + +## Files touched + +Part 1: +- `lib/features/dive_import/domain/services/dive_matcher.dart` (gate) +- `test/features/dive_import/domain/services/dive_matcher_test.dart` +- checker tests under `test/features/universal_import/...` and + `test/features/dive_import/...` + +Part 2: +- `lib/features/import_wizard/data/adapters/universal_adapter.dart` + (supported actions + `performImport` fold + `resolve()` includes consolidate) +- `lib/features/universal_import/data/services/import_duplicate_checker.dart` + (`matchedExistingSource: true` in Pass 0) +- `lib/features/universal_import/presentation/providers/import_consolidation_service.dart` + (generalize match-lookup param) + its caller + `universal_import_providers.dart` +- tests under `test/features/import_wizard/...` and + `test/features/universal_import/...` + +## Precedent + +`2026-03-17-dive-time-timezone-fix-design.md`, +`2026-03-19-multi-computer-dive-consolidation-design.md`, +`2026-03-23-unified-import-wizard-design.md`, +`2026-03-24-dive-data-source-provenance-design.md`. diff --git a/lib/features/dive_import/domain/services/dive_matcher.dart b/lib/features/dive_import/domain/services/dive_matcher.dart index 920e694ed..28240914e 100644 --- a/lib/features/dive_import/domain/services/dive_matcher.dart +++ b/lib/features/dive_import/domain/services/dive_matcher.dart @@ -23,6 +23,14 @@ class DiveMatcher { required int existingDurationSeconds, }) { final timeScore = _calculateTimeScore(wearableStartTime, existingStartTime); + + // Time is a NECESSARY condition, not just a weighted term. + // `_calculateTimeScore` is 0.0 once the two starts are >= 15 min apart; + // with no time evidence, a depth + duration coincidence (0.30 + 0.20 = + // 0.50) must not be able to reach the possible-duplicate threshold. Two + // recordings that do not line up in time cannot be the same physical dive. + if (timeScore <= 0) return 0.0; + final depthScore = _calculateDepthScore(wearableMaxDepth, existingMaxDepth); final durationScore = _calculateDurationScore( wearableDurationSeconds, diff --git a/lib/features/import_wizard/data/adapters/universal_adapter.dart b/lib/features/import_wizard/data/adapters/universal_adapter.dart index bc2183054..b4e030dbb 100644 --- a/lib/features/import_wizard/data/adapters/universal_adapter.dart +++ b/lib/features/import_wizard/data/adapters/universal_adapter.dart @@ -42,6 +42,8 @@ import 'package:submersion/features/universal_import/data/models/import_enums.da as ui; import 'package:submersion/features/universal_import/data/models/import_payload.dart'; import 'package:submersion/features/universal_import/data/services/import_duplicate_checker.dart'; +import 'package:submersion/features/universal_import/presentation/providers/import_consolidation_service.dart' + show performConsolidations; import 'package:submersion/features/universal_import/presentation/providers/universal_import_providers.dart'; import 'package:submersion/features/universal_import/presentation/widgets/field_mapping_step.dart'; import 'package:submersion/features/universal_import/presentation/widgets/file_selection_step.dart'; @@ -140,6 +142,9 @@ class UniversalAdapter implements ImportSourceAdapter { Set get supportedDuplicateActions => const { DuplicateAction.skip, DuplicateAction.importAsNew, + // File imports offer MANUAL consolidation only (no auto-consolidate: a + // file has no single "current computer" to prove a cross-computer match). + DuplicateAction.consolidate, }; @override @@ -468,11 +473,60 @@ class UniversalAdapter implements ImportSourceAdapter { cancelToken: cancelToken, ); + // Fold consolidate-flagged dives (imported as standalone above) into their + // matched existing dive. These indices come only from an explicit user + // choice in the review step, and each has a match result (the UI offers + // Consolidate only on matches). + final diveActions = + duplicateActions[wizard.ImportEntityType.dives] ?? const {}; + final consolidateIndices = { + for (final entry in diveActions.entries) + if (entry.value == DuplicateAction.consolidate) entry.key, + }; + + var consolidated = 0; + var removedDiveIds = const {}; + if (consolidateIndices.isNotEmpty) { + final matchResults = + bundle.groups[wizard.ImportEntityType.dives]?.matchResults ?? + const {}; + final summary = await performConsolidations( + indices: consolidateIndices, + diveIdByIndex: result.diveIdByIndex, + duplicateResult: ImportDuplicateResult(diveMatches: matchResults), + consolidationService: _ref.read(diveConsolidationServiceProvider), + diveRepository: repos.diveRepository, + ); + consolidated = summary.consolidated; + removedDiveIds = summary.removedDiveIds; + } + + // `importer.import` counted folded/removed dives as imported; subtract only + // the dives that were ACTUALLY removed (folded, or compensating-deleted). + // A dive whose fold AND cleanup both failed is still standalone in the DB, + // so it stays counted as imported rather than being hidden. + final counts = _convertImportCounts(result); + final netDives = result.dives - removedDiveIds.length; + if (netDives > 0) { + counts[wizard.ImportEntityType.dives] = netDives; + } else { + counts.remove(wizard.ImportEntityType.dives); + } + + final netImportedDiveIds = [ + for (final id in result.diveIds) + if (!removedDiveIds.contains(id)) id, + ]; + + // Removed-but-not-folded dives were consolidation attempts that failed and + // were cleaned up; report them as skipped (as the download adapter does). + final cleanedUpFailures = removedDiveIds.length - consolidated; + return UnifiedImportResult( - importedCounts: _convertImportCounts(result), - consolidatedCount: 0, - skippedCount: skipped, - importedDiveIds: result.diveIds, + importedCounts: counts, + consolidatedCount: consolidated, + skippedCount: skipped + cleanedUpFailures, + importedDiveIds: netImportedDiveIds, ); } @@ -713,7 +767,10 @@ class UniversalAdapter implements ImportSourceAdapter { } for (final entry in actions.entries) { - if (entry.value == DuplicateAction.importAsNew) { + // Consolidate-flagged items are imported as standalone dives first (like + // importAsNew); performImport folds them into their match afterwards. + if (entry.value == DuplicateAction.importAsNew || + entry.value == DuplicateAction.consolidate) { resolved.add(entry.key); } } diff --git a/lib/features/universal_import/data/services/import_duplicate_checker.dart b/lib/features/universal_import/data/services/import_duplicate_checker.dart index e88729e81..00e58f7d1 100644 --- a/lib/features/universal_import/data/services/import_duplicate_checker.dart +++ b/lib/features/universal_import/data/services/import_duplicate_checker.dart @@ -711,6 +711,10 @@ class ImportDuplicateChecker { score: _sourceUuidMatchScore, timeDifferenceMs: 0, siteName: existing.site?.name, + // The incoming dive's source data is already present on `existing` + // (same source_uuid) — a re-import, not a new source. The wizard + // defaults these to skip and excludes them from bulk-consolidate. + matchedExistingSource: true, ); handled.add(i); } diff --git a/lib/features/universal_import/presentation/providers/import_consolidation_service.dart b/lib/features/universal_import/presentation/providers/import_consolidation_service.dart index 200f0c2d3..6ecfa5feb 100644 --- a/lib/features/universal_import/presentation/providers/import_consolidation_service.dart +++ b/lib/features/universal_import/presentation/providers/import_consolidation_service.dart @@ -11,6 +11,7 @@ class ConsolidationSummary { const ConsolidationSummary({ required this.consolidated, required this.failed, + this.removedDiveIds = const {}, }); /// Number of dives successfully folded into their matched dive. @@ -20,6 +21,15 @@ class ConsolidationSummary { /// been imported as a standalone dive. Each one was deleted (see /// [performConsolidations]) rather than left stranded. final int failed; + + /// The freshly-imported standalone dive ids that are NO LONGER present after + /// this call -- each was either folded into its match (and tombstoned by + /// [DiveConsolidationService.apply]) or removed by the compensating delete. + /// + /// A dive whose fold AND compensating delete both failed is intentionally + /// absent: it is still standalone in the DB, so the caller must keep + /// reporting it as imported instead of hiding a stranded duplicate. + final Set removedDiveIds; } /// Folds consolidate-flagged imported dives into their matched existing @@ -50,6 +60,7 @@ Future performConsolidations({ }) async { var consolidated = 0; var failed = 0; + final removedDiveIds = {}; for (final index in indices) { final matchResult = duplicateResult?.diveMatchFor(index); @@ -64,6 +75,8 @@ Future performConsolidations({ secondaryDiveIds: [newDiveId], ); consolidated++; + // The fold tombstoned the standalone dive. + removedDiveIds.add(newDiveId); } catch (e, st) { _log.error( 'Consolidation fold failed for dive into ${matchResult.diveId}', @@ -72,10 +85,14 @@ Future performConsolidations({ ); try { await diveRepository.bulkDeleteDives([newDiveId]); + // The compensating delete removed the standalone dive. + removedDiveIds.add(newDiveId); } catch (deleteError, deleteStack) { // The compensating delete failed too -- log it and continue rather - // than rethrow, so the remaining indices are still processed - // instead of aborting the whole import. + // than rethrow, so the remaining indices are still processed instead + // of aborting the whole import. The dive is deliberately left OUT of + // [removedDiveIds] because it is still standalone in the DB; the caller + // must keep counting it as imported rather than hiding a duplicate. _log.error( 'Compensating delete failed for orphaned dive $newDiveId', error: deleteError, @@ -86,5 +103,9 @@ Future performConsolidations({ } } - return ConsolidationSummary(consolidated: consolidated, failed: failed); + return ConsolidationSummary( + consolidated: consolidated, + failed: failed, + removedDiveIds: removedDiveIds, + ); } diff --git a/test/features/dive_import/domain/services/dive_matcher_test.dart b/test/features/dive_import/domain/services/dive_matcher_test.dart index 519b6d40b..8d767498b 100644 --- a/test/features/dive_import/domain/services/dive_matcher_test.dart +++ b/test/features/dive_import/domain/services/dive_matcher_test.dart @@ -114,6 +114,69 @@ void main() { expect(score, greaterThanOrEqualTo(0.0)); expect(score, lessThanOrEqualTo(1.0)); }); + + test('gates to 0 for dives months apart even with identical ' + 'depth and duration (ScubaBoard report regression)', () { + // Months apart, both 47 min, similar depth previously scored exactly + // 0.50 = (0*0.5)+(1*0.3)+(1*0.2) and was flagged as a possible + // duplicate. Time is a necessary condition: they cannot be the same + // dive. + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 3, 15, 10, 0), + wearableMaxDepth: 18.0, + wearableDurationSeconds: 47 * 60, + existingStartTime: DateTime(2026, 6, 20, 14, 0), + existingMaxDepth: 18.0, + existingDurationSeconds: 47 * 60, + ); + + expect(score, 0.0); + expect(matcher.isPossibleDuplicate(score), isFalse); + }); + + test('gates to 0 for same-day dives hours apart with identical ' + 'depth and duration', () { + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 3, 15, 9, 0), + wearableMaxDepth: 18.0, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 3, 15, 14, 0), + existingMaxDepth: 18.0, + existingDurationSeconds: 45 * 60, + ); + + expect(score, 0.0); + }); + + test('gates to 0 exactly at the 15-minute boundary', () { + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 1, 15, 10, 0), + wearableMaxDepth: 18.5, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 1, 15, 10, 15), + existingMaxDepth: 18.5, + existingDurationSeconds: 45 * 60, + ); + + expect(score, 0.0); + }); + + test('still scores a near-in-time match within the 15-minute ' + 'window', () { + // 14 min apart: timeScore = 1 - ((14-5)/10) = 0.1 + // composite = (0.1*0.5)+(1*0.3)+(1*0.2) = 0.55 -> still a match. + final score = matcher.calculateMatchScore( + wearableStartTime: DateTime(2026, 1, 15, 10, 0), + wearableMaxDepth: 18.5, + wearableDurationSeconds: 45 * 60, + existingStartTime: DateTime(2026, 1, 15, 10, 14), + existingMaxDepth: 18.5, + existingDurationSeconds: 45 * 60, + ); + + expect(score, greaterThan(0.0)); + expect(score, closeTo(0.55, 0.0001)); + }); }); group('isProbableDuplicate', () { diff --git a/test/features/import_wizard/data/adapters/universal_adapter_test.dart b/test/features/import_wizard/data/adapters/universal_adapter_test.dart index d419590c2..f05b483e1 100644 --- a/test/features/import_wizard/data/adapters/universal_adapter_test.dart +++ b/test/features/import_wizard/data/adapters/universal_adapter_test.dart @@ -23,7 +23,10 @@ import 'package:submersion/features/dive_centers/data/repositories/dive_center_r import 'package:submersion/features/dive_centers/domain/entities/dive_center.dart'; import 'package:submersion/features/dive_centers/presentation/providers/dive_center_providers.dart'; import 'package:submersion/features/dive_import/data/services/uddf_entity_importer.dart'; +import 'package:submersion/features/dive_import/domain/services/dive_matcher.dart'; import 'package:submersion/features/dive_log/data/repositories/dive_repository_impl.dart'; +import 'package:submersion/features/dive_log/data/services/dive_consolidation_service.dart'; +import 'package:submersion/features/dive_log/data/services/dive_merge_snapshot.dart'; import 'package:submersion/features/dive_log/data/repositories/tank_pressure_repository.dart'; import 'package:submersion/features/dive_log/domain/entities/dive.dart'; import 'package:submersion/features/dive_log/presentation/providers/dive_providers.dart'; @@ -80,6 +83,7 @@ import 'package:submersion/features/universal_import/presentation/providers/univ MockSpec(), MockSpec(), MockSpec(), + MockSpec(), ]) import 'universal_adapter_test.mocks.dart'; @@ -91,6 +95,30 @@ typedef Override = riverpod.Override; final _now = DateTime.now(); +/// Minimal snapshot for a stubbed [DiveConsolidationService.apply]. The fold +/// is exercised for real by dive_consolidation_service_test and the download +/// adapter's consolidate integration test; here we only assert the adapter's +/// wiring (that it delegates to apply and adjusts its counts). +const _emptySnapshot = DiveMergeSnapshot( + mergedDiveId: 'target-dive', + diveRows: [], + profileRows: [], + tankRows: [], + weightRows: [], + customFieldRows: [], + equipmentRows: [], + diveTypeRows: [], + tagRows: [], + buddyRows: [], + sightingRows: [], + eventRows: [], + gasSwitchRows: [], + tankPressureRows: [], + dataSourceRows: [], + tideRows: [], + mediaDiveIds: {}, +); + Diver _testDiver() => Diver(id: 'diver-1', name: 'Test Diver', createdAt: _now, updatedAt: _now); @@ -319,20 +347,19 @@ void main() { ); }); - testWidgets('supportedDuplicateActions contains skip and importAsNew', ( - tester, - ) async { + testWidgets('supportedDuplicateActions includes skip, importAsNew, and ' + 'consolidate', (tester) async { await _runWithAdapter( tester, overrides: _buildBundleOverrides(), callback: (adapter) async { expect( adapter.supportedDuplicateActions, - containsAll([DuplicateAction.skip, DuplicateAction.importAsNew]), - ); - expect( - adapter.supportedDuplicateActions, - isNot(contains(DuplicateAction.consolidate)), + containsAll([ + DuplicateAction.skip, + DuplicateAction.importAsNew, + DuplicateAction.consolidate, + ]), ); }, ); @@ -425,6 +452,172 @@ void main() { }); }); + // ------------------------------------------------------------------------- + // performImport -- consolidate + // ------------------------------------------------------------------------- + + group('performImport() - consolidate', () { + testWidgets('folds a consolidate-flagged dive into its matched dive and ' + 'reports it as consolidated, not imported', (tester) async { + final mockConsolidation = MockDiveConsolidationService(); + when( + mockConsolidation.apply( + targetDiveId: anyNamed('targetDiveId'), + secondaryDiveIds: anyNamed('secondaryDiveIds'), + ), + ).thenAnswer( + (invocation) async => DiveConsolidationOutcome( + targetDiveId: invocation.namedArguments[#targetDiveId] as String, + snapshot: _emptySnapshot, + ), + ); + + final payload = ImportPayload( + entities: { + ui.ImportEntityType.dives: [ + { + 'dateTime': DateTime(2026, 7, 1, 9, 0), + 'maxDepth': 24.5, + 'runtime': const Duration(minutes: 40), + }, + ], + }, + ); + + await _runWithAdapter( + tester, + overrides: [ + ..._fullOverrides(payload: payload, diver: _testDiver()), + diveConsolidationServiceProvider.overrideWithValue(mockConsolidation), + ], + callback: (adapter) async { + final base = await adapter.buildBundle(); + // Flag the single incoming dive as a duplicate of an existing dive + // and mark it for consolidation, mirroring what the review step + // hands to performImport. + final bundle = ImportBundle( + source: base.source, + groups: { + wizard.ImportEntityType.dives: EntityGroup( + items: base.groups[wizard.ImportEntityType.dives]!.items, + duplicateIndices: const {0}, + matchResults: const { + 0: DiveMatchResult( + diveId: 'target-dive', + score: 0.9, + timeDifferenceMs: 0, + ), + }, + ), + }, + ); + + final result = await adapter.performImport( + bundle, + { + wizard.ImportEntityType.dives: {0}, + }, + { + wizard.ImportEntityType.dives: {0: DuplicateAction.consolidate}, + }, + ); + + expect(result.consolidatedCount, equals(1)); + // The folded dive was imported as a standalone then tombstoned by the + // fold, so it must NOT also be counted as a new imported dive. + expect( + result.importedCounts[wizard.ImportEntityType.dives] ?? 0, + equals(0), + ); + verify( + mockConsolidation.apply( + targetDiveId: 'target-dive', + secondaryDiveIds: anyNamed('secondaryDiveIds'), + ), + ).called(1); + }, + ); + }); + + testWidgets('reports a stranded dive as imported when both the fold and ' + 'its cleanup fail (does not hide it)', (tester) async { + final mockConsolidation = MockDiveConsolidationService(); + when( + mockConsolidation.apply( + targetDiveId: anyNamed('targetDiveId'), + secondaryDiveIds: anyNamed('secondaryDiveIds'), + ), + ).thenThrow(ArgumentError('fold failed')); + + // The compensating delete also fails, so the standalone dive cannot be + // removed and is left stranded in the DB. + final mockDiveRepo = MockDiveRepository(); + when(mockDiveRepo.bulkDeleteDives(any)).thenThrow(Exception('delete')); + + final payload = ImportPayload( + entities: { + ui.ImportEntityType.dives: [ + { + 'dateTime': DateTime(2026, 7, 1, 9, 0), + 'maxDepth': 24.5, + 'runtime': const Duration(minutes: 40), + }, + ], + }, + ); + + await _runWithAdapter( + tester, + overrides: [ + ..._fullOverrides( + payload: payload, + diver: _testDiver(), + mockDiveRepo: mockDiveRepo, + ), + diveConsolidationServiceProvider.overrideWithValue(mockConsolidation), + ], + callback: (adapter) async { + final base = await adapter.buildBundle(); + final bundle = ImportBundle( + source: base.source, + groups: { + wizard.ImportEntityType.dives: EntityGroup( + items: base.groups[wizard.ImportEntityType.dives]!.items, + duplicateIndices: const {0}, + matchResults: const { + 0: DiveMatchResult( + diveId: 'target-dive', + score: 0.9, + timeDifferenceMs: 0, + ), + }, + ), + }, + ); + + final result = await adapter.performImport( + bundle, + { + wizard.ImportEntityType.dives: {0}, + }, + { + wizard.ImportEntityType.dives: {0: DuplicateAction.consolidate}, + }, + ); + + // The fold failed AND the standalone could not be deleted, so the + // dive is still present -- it must be reported as imported, not + // silently hidden. + expect(result.consolidatedCount, equals(0)); + expect( + result.importedCounts[wizard.ImportEntityType.dives] ?? 0, + equals(1), + ); + }, + ); + }); + }); + // ------------------------------------------------------------------------- // buildBundle -- null payload // ------------------------------------------------------------------------- diff --git a/test/features/universal_import/data/services/import_duplicate_checker_test.dart b/test/features/universal_import/data/services/import_duplicate_checker_test.dart index 91ec5d2ef..4e1ab164d 100644 --- a/test/features/universal_import/data/services/import_duplicate_checker_test.dart +++ b/test/features/universal_import/data/services/import_duplicate_checker_test.dart @@ -452,6 +452,65 @@ void main() { expect(result.diveMatches, contains(0)); }); + + test('content (fuzzy) match leaves matchedExistingSource false', () { + final diveTime = DateTime(2024, 1, 15, 10, 0); + final result = checkWith( + payload: ImportPayload( + entities: { + ImportEntityType.dives: [ + { + 'dateTime': diveTime, + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 45), + }, + ], + }, + ), + dives: [ + Dive( + id: 'existing-1', + dateTime: diveTime.add(const Duration(minutes: 2)), + maxDepth: 18.0, + bottomTime: const Duration(minutes: 45), + ), + ], + ); + + final match = result.diveMatchFor(0); + expect(match, isNotNull); + expect(match!.matchedExistingSource, isFalse); + }); + + test('does not match a dive far apart in time even with identical ' + 'depth and duration', () { + // ScubaBoard regression at the checker level: months apart, same depth, + // same duration. Only the DiveMatcher time-gate keeps this from being + // flagged as a possible duplicate. + final result = checkWith( + payload: ImportPayload( + entities: { + ImportEntityType.dives: [ + { + 'dateTime': DateTime(2024, 3, 15, 10, 0), + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 47), + }, + ], + }, + ), + dives: [ + Dive( + id: 'existing-1', + dateTime: DateTime(2024, 6, 20, 14, 0), + maxDepth: 18.0, + bottomTime: const Duration(minutes: 47), + ), + ], + ); + + expect(result.diveMatches, isEmpty); + }); }); group('Dive duplicates (source_uuid)', () { @@ -619,6 +678,37 @@ void main() { // No UUID match (both empty), and content doesn't match either. expect(result.diveMatches, isEmpty); }); + + test('source_uuid match flags matchedExistingSource so it defaults ' + 'to skip', () { + final result = checkWith( + payload: ImportPayload( + entities: { + ImportEntityType.dives: [ + { + 'sourceUuid': 'XYZ', + 'dateTime': DateTime(2024, 1, 15, 10, 0), + 'maxDepth': 18.0, + 'runtime': const Duration(minutes: 45), + }, + ], + }, + ), + dives: [ + Dive( + id: 'existing-1', + dateTime: DateTime(2024, 1, 15, 10, 0), + maxDepth: 18.0, + bottomTime: const Duration(minutes: 45), + ), + ], + existingSourceUuidByDiveId: const {'existing-1': 'XYZ'}, + ); + + final match = result.diveMatchFor(0); + expect(match, isNotNull); + expect(match!.matchedExistingSource, isTrue); + }); }); group('Empty payload', () { diff --git a/test/features/universal_import/presentation/providers/import_consolidation_service_test.dart b/test/features/universal_import/presentation/providers/import_consolidation_service_test.dart index 7c0e6b578..10660fa43 100644 --- a/test/features/universal_import/presentation/providers/import_consolidation_service_test.dart +++ b/test/features/universal_import/presentation/providers/import_consolidation_service_test.dart @@ -168,6 +168,7 @@ void main() { expect(summary.consolidated, 1); expect(summary.failed, 0); + expect(summary.removedDiveIds, {'new-dive-1'}); verify( mockConsolidationService.apply( targetDiveId: 'existing-dive-1', @@ -199,6 +200,7 @@ void main() { ); expect(summary.consolidated, 2); + expect(summary.removedDiveIds, {'new-dive-0', 'new-dive-1'}); verify( mockConsolidationService.apply( targetDiveId: 'existing-dive-a', @@ -232,6 +234,7 @@ void main() { ); expect(summary.consolidated, 1); + expect(summary.removedDiveIds, {'new-dive-0'}); verify( mockConsolidationService.apply( targetDiveId: 'existing-dive-a', @@ -284,6 +287,8 @@ void main() { // the exception from index 0 must not abort the loop. expect(summary.consolidated, 1); expect(summary.failed, 1); + // Both are gone: new-dive-0 was compensating-deleted, new-dive-1 folded. + expect(summary.removedDiveIds, {'new-dive-0', 'new-dive-1'}); verify(mockDiveRepository.bulkDeleteDives(['new-dive-0'])).called(1); verifyNever(mockDiveRepository.bulkDeleteDives(['new-dive-1'])); verify( @@ -334,6 +339,11 @@ void main() { // still gets processed and succeeds. expect(summary.consolidated, 1); expect(summary.failed, 1); + // new-dive-0's fold AND its compensating delete both failed, so it is + // still standalone -- it must NOT be reported as removed, which is what + // keeps the caller from hiding a stranded duplicate from the summary. + expect(summary.removedDiveIds, {'new-dive-1'}); + expect(summary.removedDiveIds, isNot(contains('new-dive-0'))); verify(mockDiveRepository.bulkDeleteDives(['new-dive-0'])).called(1); verify( mockConsolidationService.apply(