Skip to content

Fix MacDive UDDF import (Milestone 1 of 4)#252

Merged
ericgriffin merged 22 commits into
mainfrom
feature/macdive-uddf-gap-fill
Apr 23, 2026
Merged

Fix MacDive UDDF import (Milestone 1 of 4)#252
ericgriffin merged 22 commits into
mainfrom
feature/macdive-uddf-gap-fill

Conversation

@ericgriffin
Copy link
Copy Markdown
Member

@ericgriffin ericgriffin commented Apr 21, 2026

Summary

First of four planned milestones addressing MacDive import issues (closes #178 in part, foundational for #179).

  • Fixed: gear doesn't import from MacDive UDDF. Parser was only scanning Submersion's private <applicationdata><submersion><equipment> extension, missing the standard UDDF gear location at <diver><owner><equipment> where MacDive and every other compliant exporter puts it. Now extracts from both locations (dedup'd by source UUID). Real-sample test confirms 29/29 equipment items land.

  • Fixed: gas switches for deco dives dropped at import. <switchmix ref> markers on waypoints updated internal state but never populated the sample map. Fixed at the parser; persistence to dive_profile_samples deferred to M1.5 (needs an events-table design decision).

  • Fixed: <equipmentused><link ref> only captured from one section. Now captured from both informationbeforedive and informationafterdive, with dedup.

  • Fixed: <surfaceintervalbeforedive><infinity/></…> (MacDive first-dive marker) is now explicitly handled rather than relying on silent int.tryParse failure.

  • Added: richer dive metadata extraction + persistence. Schema bumped 69 → 70 → 71. New columns on dives (boat_name, boat_captain, dive_operator, surface_conditions) and dive_sites (water_type, body_of_water). Weather flows into the existing weather_description column; site difficulty into the existing difficulty column. dive_number_of_day was originally planned as a stored column but dropped in favor of deriving it on demand from diveDateTime (no current reader and self-healing as manually-logged dives are interleaved with imports).

  • Added: cross-format import deduplication via source_uuid on the existing dive_data_sources sidecar. Preserves stable per-dive IDs from MacDive, Shearwater Cloud, and Subsurface SSRF. Re-importing the same dives in a different format no longer creates duplicates.

  • Added: gated real-sample regression test (@Tags(['real-data'])) against the 29MB MacDive UDDF export, asserting 540 dives, 350+ sites, 30+ buddies, 20+ gear, and spot-checks for sourceUuid, equipmentRefs, gasMixRef, and country-nesting.

Dropped from scope (parse-side extraction removed)

  • personalMode, altitudeMode, signature, site flag — niche MacDive-specific concepts or redundant with existing columns (we already have numeric altitude).
  • LinkRefKind / LinkRefIndex utility types — added in support of a bug that turned out not to exist in the codebase (the parser already uses map-based ref disambiguation, not positional).

Deferred to future milestones

  • Profile gasMixRef persistence (parsed in memory; needs events-table design).
  • M2 adds MacDive native XML import (gets us tags, which UDDF doesn't emit).
  • M3 adds MacDive SQLite import (complete path including critters, events, photos).
  • M4 adds photo import across all three formats.

Commit narrative

17 commits tell the story top-to-bottom: schema foundation → parser utilities → per-field extractions → bug fixes → integration tests → cleanup. Each commit is individually reviewable.

Notable: Task 3 was investigated and explicitly skipped after the implementer discovered the diagnosed bug didn't exist in the code. Task 7 was descoped based on user guidance (only preserve stable IDs for sources that actually emit them — which led to the narrow dive_data_sources.source_uuid design instead of 9 new columns).

Test plan

  • flutter test — 7112 tests pass
  • flutter analyze — clean
  • dart format — clean
  • Migration v69 → v70 tested (test/core/database/migration_v70_test.dart)
  • Migration v70 → v71 tested (test/core/database/migration_v71_test.dart)
  • End-to-end UDDF import → DB tested for source_uuid and MacDive extended fields
  • Gated real-sample test passes against user's 29MB MacDive UDDF export
  • Manual verification: install this branch over existing data, confirm migrations run cleanly and don't corrupt existing dives
  • Manual verification: import the user's MacDive UDDF and confirm dives show with site water type, boat info, gear, etc.

User-reported issues closed

  • glazerama (ScubaBoard #296): MacDive UDDF missing tags → not fully addressed here (MacDive UDDF genuinely doesn't emit tags; M2 closes this via the XML exporter which does)
  • glazerama (#296): MacDive .xml format not recognized → deferred to M2
  • Josh the diver (#297): gear didn't import → ✅ closed
  • Josh the diver (#297): gas switches for deco not imported → ✅ closed at parser; persistence deferred to M1.5
  • GH Fully support importing MacDive UDDF and XML files #178: MacDive UDDF importer has field gaps → ✅ closed for the 6 stored fields + 1 derived (dive_number_of_day); 4 niche fields explicitly dropped

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Milestone 1 for improving MacDive UDDF import support by expanding UDDF parsing (standard gear location, additional dive/site metadata, gas-switch markers, source UUID capture) and persisting new metadata to the database with schema migrations and regression tests.

Changes:

  • Extend UDDF parsing to capture MacDive/standard UDDF fields (gear under <diver><owner><equipment>, <dive id> as sourceUuid, richer dive/site metadata, improved equipment refs, gas-switch markers).
  • Persist new MacDive metadata into new DB columns and persist per-dive sourceUuid into dive_data_sources.source_uuid; bump schema to v71 with v70/v71 migrations + tests.
  • Add integration and real-sample regression tests; introduce a real-data test tag default-skipped via dart_test.yaml.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/features/dive_import/uddf_import_source_uuid_test.dart New integration test verifying dive_data_sources.source_uuid persistence for UDDF <dive id>.
test/features/dive_import/uddf_import_macdive_fields_test.dart New integration tests verifying MacDive metadata persistence into new dives/dive_sites columns.
test/core/services/export/uddf/uddf_macdive_real_sample_test.dart New gated “real-data” regression suite against a large MacDive UDDF export.
test/core/services/export/uddf/uddf_macdive_import_test.dart Adds unit tests validating new MacDive/UDDF parsing behaviors (metadata, equipment refs, gear, switchmix markers, infinity surface interval).
test/core/database/migration_v71_test.dart New migration tests ensuring v71 columns exist and upgrade v70→v71 is idempotent.
test/core/database/migration_v70_test.dart New migration tests ensuring source_uuid exists and upgrade v69→v70 works.
lib/features/dive_sites/data/repositories/site_repository_impl.dart Adds repository method to apply importer-only site column updates and mark pending sync.
lib/features/dive_log/data/repositories/dive_repository_impl.dart Adds repository method to apply importer-only dive column updates and mark pending sync.
lib/features/dive_import/data/services/uddf_entity_importer.dart Persists MacDive parsed metadata into new DB columns; persists sourceUuid into dive_data_sources.
lib/core/services/export/uddf/uddf_full_import_service.dart Enhances UDDF parsing for standard gear, site/dive source UUIDs, richer metadata, equipment refs, and switchmix sample markers.
lib/core/domain/models/incoming_dive_data.dart Extends normalized incoming-dive model to carry new MacDive metadata and sourceUuid.
lib/core/database/database.dart Adds new columns + migrations (v70 source_uuid, v71 MacDive metadata columns) and bumps schema version to 71.
docs/superpowers/plans/2026-04-21-macdive-uddf-gap-fill.md Updates milestone plan/status documentation to reflect landed scope and rationale.
dart_test.yaml Adds default-skipped real-data tag configuration.
CHANGELOG.md Adds Unreleased changelog entries for MacDive UDDF improvements and dedup behavior.

Comment thread lib/features/dive_import/data/services/uddf_entity_importer.dart Outdated
Comment thread lib/features/dive_import/data/services/uddf_entity_importer.dart Outdated
Comment thread test/core/services/export/uddf/uddf_macdive_real_sample_test.dart Outdated
Comment thread test/core/services/export/uddf/uddf_macdive_real_sample_test.dart Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

ericgriffin added a commit that referenced this pull request Apr 22, 2026
Drift's Value(null) sets the column to NULL - it does NOT mean
'leave this field untouched'. Re-importing a MacDive UDDF whose
new data was missing fields the existing row already had would
silently wipe those fields. All nullable-field writes to
DivesCompanion and DiveSitesCompanion now use Value.absent()
when the source value is null, preserving existing data on
partial re-imports.

Addresses Copilot review comments 1 and 2 on PR #252.
ericgriffin added a commit that referenced this pull request Apr 22, 2026
- Path is now read from MACDIVE_UDDF_SAMPLE (compile-time env var
  set via --dart-define). No more hard-coded developer username.
- Each test checks hasFixture and calls markTestSkipped/return
  if the sample isn't available. --run-skipped --tags=real-data
  now cleanly skips rather than crashing on uninitialized state.
- Added top-of-file comment documenting the env-var workflow.

Addresses Copilot review comments 3 and 4 on PR #252.
@ericgriffin ericgriffin moved this from Backlog to In review in Submersion Release Tracker Apr 22, 2026
@ericgriffin ericgriffin self-assigned this Apr 22, 2026
Copilot AI review requested due to automatic review settings April 23, 2026 00:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Comment thread lib/core/database/database.dart
Comment thread docs/superpowers/plans/2026-04-21-macdive-uddf-gap-fill.md Outdated
Copilot AI review requested due to automatic review settings April 23, 2026 00:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Comment thread docs/superpowers/plans/2026-04-21-macdive-uddf-gap-fill.md
Matches existing per-source architecture (raw_fingerprint,
source_format, source_file_name, computer_serial already live
on dive_data_sources). One column, one table, one migration.
Non-dive entities stay on content-based dedup for now.
- Rename to migration_v70_test.dart matching v58/v66/v68/v69 pattern
- Use addTearDown(db.close) for leak-safe cleanup
- Add v69 -> v70 migration-path test seeding pre-migration schema
Pre-cursor to Task 3 which will replace positional <link ref>
resolution in informationbeforedive with kind-based binding.
Investigation showed UddfFullImportService already uses map-based
link-ref disambiguation, not positional order. Prescribed test
passes on unmodified code. Documented the finding and the real
(unrelated) gap for trip/center/course UUIDs which will surface
in Milestone 3 if at all.
Verifies that MacDive's <infinity/> first-dive marker does not
produce a spurious surfaceInterval=0. The current parser handles
this correctly via int.tryParse; this test is a regression guard.
…ator, ...) and source UUID

Adds 10 new optional fields that MacDive emits but the parser
previously ignored: weather, surfaceConditions, boatName,
boatCaptain, diveOperator, personalMode, altitudeMode, signature,
diveNumberOfDay, and the dive element's id attribute as sourceUuid.
Threaded through IncomingDiveData.
…and source UUID

MacDive emits these site-level fields but the current parser
ignored them. Extract into the site data map alongside name,
country, and coordinates.
…are captured

Previously only scanned informationbeforedive for <equipmentused> refs.
Now merges refs from both informationbeforedive and informationafterdive
into equipmentRefs, supporting both <equipmentref> elements and
<link ref="..."/> attributes.
MacDive emits <switchmix ref="gas-UUID"/> inside waypoints to mark
gas changes during a dive; the parser now captures this ref as gasMixRef
on the sample map so downstream consumers (deco display, gas stats) can
see the change event.

Regression guard test added to prevent future breaks of this feature
per the ScubaBoard 'gas switches for deco not imported' complaint.
Closes the pipe from parser to DB for the source UUID plumbing
added in Tasks 1 and 5. MacDive, Shearwater Cloud, and any other
source with a stable per-dive ID can now round-trip through import
and be matched on re-import.

Other dive-level MacDive fields (weather, boat, operator, …)
remain in the dive map but have no persistence target yet;
deferred to a later milestone that adds display for them.
… source_uuid

Previous test re-exercised Task 5's UddfFullImportService parser
but never hit UddfEntityImporter, leaving the one-line DB write
at line 1425 untested. This test now spins up an in-memory
AppDatabase via setUpTestDatabase, runs UddfEntityImporter.import
with real repositories, and queries dive_data_sources.source_uuid
directly to verify both the populated and null-attribute cases.
Runs the user's 29MB MacDive UDDF through the parser and asserts
dive/site/buddy/gear counts plus spot-checks for sourceUuid,
equipmentRefs, gasMixRef, and the country-nesting dialect fix.

Gated behind @tags(['real-data']) so CI stays fast.
…location

MacDive and other UDDF-compliant exporters emit gear under
<diver><owner><equipment> with child elements like <variouspieces>,
<suit>, <divecomputer>, <regulator>, <bcd>. The parser previously
only scanned Submersion's private <applicationdata><submersion>
extension, so real-world UDDF imports produced zero gear - the
root cause of the 'my gear didn't import' complaint.

Now scans both locations, deduping by source UUID. Real-sample
test restored to assert >=20 items.
Removes parser extraction and IncomingDiveData fields for MacDive
fields we've decided not to persist: personalMode, altitudeMode,
signature, site flag. These are niche MacDive-specific concepts or
redundant with existing columns.

Also removes LinkRefKind and LinkRefIndex — added for a bug that
turned out not to exist (Task 3 investigation) and never wired up.
Adds 7 new columns on dives and dive_sites for previously
extract-but-discard fields:
- dives: dive_number_of_day, boat_name, boat_captain, dive_operator,
  surface_conditions
- dive_sites: water_type, body_of_water

Also wires the existing weather_description column for UDDF imports
(populated from MacDive <weather>). The importer calls narrow new
applyImportedMetadata helpers on DiveRepository and SiteRepository so
the new columns ride along with the existing repository-created rows
without needing entity field churn (copyWith, equatable, sync serialiser).

Schema v70 -> v71 with an idempotent migration guarded by PRAGMA
introspection, matching the v69/v70 house style. Closes the parser-to-DB
gap for the MacDive extended fields extracted in Milestone 1 Tasks 5-6.
Drift's Value(null) sets the column to NULL - it does NOT mean
'leave this field untouched'. Re-importing a MacDive UDDF whose
new data was missing fields the existing row already had would
silently wipe those fields. All nullable-field writes to
DivesCompanion and DiveSitesCompanion now use Value.absent()
when the source value is null, preserving existing data on
partial re-imports.

Addresses Copilot review comments 1 and 2 on PR #252.
- Path is now read from MACDIVE_UDDF_SAMPLE (compile-time env var
  set via --dart-define). No more hard-coded developer username.
- Each test checks hasFixture and calls markTestSkipped/return
  if the sample isn't available. --run-skipped --tags=real-data
  now cleanly skips rather than crashing on uninitialized state.
- Added top-of-file comment documenting the env-var workflow.

Addresses Copilot review comments 3 and 4 on PR #252.
Earlier commits on this branch added DiveRepository.applyImportedMetadata
and SiteRepository.applyImportedMetadata without regenerating the
dependent mockito mocks. This picks up that drift across 9 .mocks.dart
files.
dive_number_of_day had zero readers -- parsed from <divenumberofday>,
stored, never queried. It's trivially derivable from dive_date_time via
ROW_NUMBER() OVER (PARTITION BY DATE(dive_date_time) ORDER BY
dive_date_time), which self-heals when dives are added manually
between imported ones.

Removes the column from the v71 migration (PR not merged yet, safe to
amend), the schema definition, IncomingDiveData, the UDDF parser, and
the entity importer. Updates CHANGELOG and the three tests that
referenced it.
Copilot review flagged drift between the plan's status section ("7 new
columns including dive_number_of_day") and the actual v71 schema (6
columns, no dive_number_of_day). Status section now lists only the
columns that shipped; a new "Post-completion revision" subsection
documents why dive_number_of_day was dropped (trivially derivable from
diveDateTime, no reader, desyncs on manually-interleaved dives). Deeper
task sections are left as historical record of the original plan.
@ericgriffin ericgriffin force-pushed the feature/macdive-uddf-gap-fill branch from f8994f0 to 90b3c44 Compare April 23, 2026 01:35
@ericgriffin ericgriffin merged commit 04179a7 into main Apr 23, 2026
16 checks passed
ericgriffin added a commit that referenced this pull request Apr 23, 2026
Drift's Value(null) sets the column to NULL - it does NOT mean
'leave this field untouched'. Re-importing a MacDive UDDF whose
new data was missing fields the existing row already had would
silently wipe those fields. All nullable-field writes to
DivesCompanion and DiveSitesCompanion now use Value.absent()
when the source value is null, preserving existing data on
partial re-imports.

Addresses Copilot review comments 1 and 2 on PR #252.
ericgriffin added a commit that referenced this pull request Apr 23, 2026
- Path is now read from MACDIVE_UDDF_SAMPLE (compile-time env var
  set via --dart-define). No more hard-coded developer username.
- Each test checks hasFixture and calls markTestSkipped/return
  if the sample isn't available. --run-skipped --tags=real-data
  now cleanly skips rather than crashing on uninitialized state.
- Added top-of-file comment documenting the env-var workflow.

Addresses Copilot review comments 3 and 4 on PR #252.
@github-project-automation github-project-automation Bot moved this from In review to Done in Submersion Release Tracker Apr 23, 2026
@ericgriffin ericgriffin deleted the feature/macdive-uddf-gap-fill branch April 23, 2026 02:14
ericgriffin added a commit that referenced this pull request Apr 23, 2026
PR #252 (UDDF gap-fill M1) landed on main and was followed by two
refactors that this branch needs to absorb:

- `dive_number_of_day` column removed (derivable from `diveDateTime`).
  Dropped from `database.dart`, `IncomingDiveData`, `uddf_entity_importer`
  write path, `uddf_full_import_service` parser, and the MacDive XML
  parser's tank-map output. Removed assertions from schema + integration
  tests.
- `Value.absent()` for missing companion fields in `uddf_entity_importer`
  applyImportedMetadata paths (site + dive).

Also swapped the hard-coded real-sample path in
`uddf_macdive_real_sample_test.dart` for main's env-var / skip-safe
version so the suite runs cleanly on CI and fresh clones.

Tests: 1539 passing, 5 skipped (gated real-data suites).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Fully support importing MacDive UDDF and XML files

2 participants