Add MacDive native XML import (Milestone 2 of 4)#254
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for importing MacDive’s native .xml logbook format into the Universal Import pipeline, primarily to preserve MacDive tags (which MacDive UDDF exports don’t include) and to normalize Imperial/Metric values at the parsing boundary.
Changes:
- Introduces
ImportFormat.macdiveXml, adds format detection (DOCTYPE +<dives>/<schema>fallback), and wires the parser intoUniversalImportNotifier. - Adds MacDive XML ingestion pipeline: typed XML reader/models + unit converter + value mapper +
MacDiveXmlParsermapping to the unifiedImportPayload. - Adds fixtures + unit tests + real-sample gated regression coverage; updates mocks/changelog/docs accordingly.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/features/universal_import/data/models/import_enums.dart | Adds macdiveXml format + source override option. |
| lib/features/universal_import/data/services/format_detector.dart | Detects MacDive XML before generic UDDF detection. |
| lib/features/universal_import/presentation/providers/universal_import_providers.dart | Wires ImportFormat.macdiveXml to MacDiveXmlParser. |
| lib/features/universal_import/data/services/macdive_xml_models.dart | Defines typed models for parsed MacDive XML. |
| lib/features/universal_import/data/services/macdive_unit_converter.dart | Converts Imperial → SI canonical units at the reader boundary. |
| lib/features/universal_import/data/services/macdive_value_mapper.dart | Maps MacDive raw strings to Submersion enums / normalized values. |
| lib/features/universal_import/data/services/macdive_xml_reader.dart | Parses XML into typed models using canonical SI units. |
| lib/features/universal_import/data/parsers/macdive_xml_parser.dart | Maps typed logbook into ImportPayload with inline dedup. |
| test/fixtures/macdive_xml/metric_small.xml | Metric fixture for reader/parser tests. |
| test/fixtures/macdive_xml/imperial_small.xml | Imperial fixture for conversion tests. |
| test/features/universal_import/data/services/macdive_xml_reader_test.dart | Validates XML parsing + edge cases + unit conversion behavior. |
| test/features/universal_import/data/services/macdive_unit_converter_test.dart | Unit tests for conversion math and null-handling. |
| test/features/universal_import/data/services/macdive_value_mapper_test.dart | Unit tests for enum/string mapping logic. |
| test/features/universal_import/data/parsers/macdive_xml_parser_test.dart | Parser-level tests for entities, refs, and basic mapping. |
| test/features/universal_import/data/parsers/macdive_xml_real_sample_test.dart | Gated real-sample regression coverage for large XML. |
| test/features/universal_import/data/services/format_detector_test.dart | Adds detector tests for MacDive XML cases. |
| test/features/universal_import/presentation/providers/universal_import_notifier_test.dart | Adds notifier-level detection test and direct parser payload test. |
| test/features/universal_import/data/models/import_enums_test.dart | Updates enum counts and displayName/isSupported expectations. |
| test/features/weather/data/repositories/weather_repository_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/universal_import/presentation/providers/import_consolidation_service_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/import_wizard/data/adapters/universal_adapter_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/import_wizard/data/adapters/healthkit_adapter_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/import_wizard/data/adapters/dive_computer_adapter_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/import_wizard/data/adapters/dive_computer_adapter_reimport_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/dive_import/presentation/providers/dive_import_notifier_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/dive_import/data/services/uddf_entity_importer_test.mocks.dart | Regenerated mocks for new repository API surface. |
| test/features/dive_computer/data/services/dive_import_service_test.mocks.dart | Regenerated mocks for new repository API surface. |
| docs/superpowers/plans/2026-04-21-macdive-native-xml-import.md | Updates milestone status and implementation notes. |
| CHANGELOG.md | Documents new MacDive XML import capability and source override. |
This was referenced Apr 22, 2026
ericgriffin
added a commit
that referenced
this pull request
Apr 23, 2026
- Align MacDive tank map with UddfEntityImporter: `volume` / `workingPressure` keys and `GasMix` object (importer does `t['gasMix'] as GasMix?`, so the prior Map would throw a TypeError and `volumeL` / `workingPressureBar` were silently dropped). - Return UTC wall-time from `MacDiveXmlReader._parseDate` so dive timestamps don't drift with device timezone / DST, matching the Subsurface parser convention. - Relax MacDive format detection to match `<dives` / `<schema` as prefixes so root tags with attributes / namespaces still detect. - Skip empty `<item/>` gear elements instead of producing a phantom `"||"` equipment entity. - Use `buddyRefs` (resolved via `buddyIdMapping`) instead of `unmatchedBuddyNames` so deselected buddies don't get created unconditionally — mirrors SubsurfaceXmlParser.
ericgriffin
force-pushed
the
feature/macdive-uddf-gap-fill
branch
from
f8994f0 to
90b3c44
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
ericgriffin
added a commit
that referenced
this pull request
Apr 23, 2026
- MacDiveXmlReader now rejects non-`<dives>` root elements with a clear FormatException, instead of silently producing an empty logbook when a user forces the MacDive XML source onto a UDDF or other file. The parser's try/catch converts the exception into a user-visible ImportWarning. - MacDive XML real-sample test replaces the hardcoded `/Users/ericgriffin/...` path with a `MACDIVE_XML_SAMPLE` compile- time env var and skip-safe fallback, mirroring the UDDF sample test. CI and fresh clones now stay green. - MacDive XML parser links imported gear back onto each dive via `equipmentRefs` keyed by the composite `(manufacturer|name|serial)` uddfId, so dives actually reference the equipment entities created from their `<gear>` children. Added tests covering both the normal case and the empty-item path (no phantom refs).
ericgriffin
added a commit
that referenced
this pull request
Apr 23, 2026
Addresses the PR #254 round-3 review: - `dive_tags` has no UNIQUE(diveId, tagId) constraint, so duplicate `<tag>` entries on one dive would insert duplicate junction rows (tag shown twice on the imported dive). Dedup `tagRefs` before emitting on the dive map. - `addBuddyToDive` already upserts, so duplicate `<buddy>` entries won't create junk rows, but the redundant DB round-trips are wasted work. Apply the same list-contains dedup used for `equipmentRefs`. - Use the existing `MacDiveValueMapper.rating` helper instead of re-implementing clamp/round inline, keeping the rating rules consistent with the planned MacDive SQLite importer. Adds two regression tests covering duplicate `<buddy>` and `<tag>` entries in a single dive.
M2 Task 1 — prepares the enum surface for the upcoming MacDive native XML parser. Parser and detector land in later tasks.
Two-branch detection: primary DOCTYPE URL match plus fallback on <dives>+<schema> root structure. Placed before the UDDF branch in the chain because the two are distinct XML formats.
Pure data classes the reader (Task 6) populates and the parser (Task 8) consumes. Numeric fields in SI canonical units; unit converter (Task 5) handles Imperial/Metric at the reader boundary. MacDive site records are per-dive (no top-level site list); the parser dedups by name when building ImportPayload entities. MacDive's lat=0 lon=0 convention for 'no GPS set' is applied at the reader boundary, so MacDiveXmlSite carries null for both when that's the case.
Feet → meters, Fahrenheit → Celsius, PSI → bar, pounds → kg, and imperial cubic-feet-at-working-pressure → liters water capacity. Applied at the MacDive XML reader boundary so all downstream code sees one unit system. unknown unit system passes through unchanged — best effort when the MacDive <units> declaration is missing or unparseable.
…odels Converts <dives> root + nested <dive>/<site>/<gear>/<gases>/<samples> into MacDiveXmlLogbook. All numeric fields are normalised to SI canonical units at this reader boundary via MacDiveUnitConverter. Edge cases handled: lat=0+lon=0 -> null GPS; empty elements -> null; missing optional children -> null; unknown units system -> passthrough.
Proves the unit converter is invoked at the reader boundary for every numeric field: dive depths, temps, weight, gas pressures, tank size (cft-at-pressure → L), sample depths/temps/pressures. Plain passthrough of seconds-denominated fields (sample time, duration) is verified too.
Maps MacDive native XML's per-dive site/gear/gas/tag inline records into the unified payload shape with dedup by name (sites, buddies, tags) and by (manufacturer|name|serial) for gear. Dive map keys match the UDDF parser so the downstream importer (UddfEntityImporter) consumes both sources without code changes. Wire-up into _parserFor lands in Task 9.
Tolerant case-insensitive substring matching for MacDive XML's categorical string fields. Unknown values return null so the importer omits the field rather than writing a misleading default. Shared between M2 (MacDive XML) and M3 (MacDive SQLite) parsers.
…pper
Parser previously passed these as raw MacDive strings ('saltwater',
'Boat') straight into the dive/site maps, but the downstream
UddfEntityImporter._parseEnum matches by canonical enum name
('salt', 'boat'). Raw strings therefore silently became null.
Now uses MacDiveValueMapper to translate to the canonical enum and
emits the .name — _parseEnum resolves it correctly. Unknown values
continue to omit the key entirely (importer keeps its default).
Adds the ImportFormat.macdiveXml case to UniversalImportNotifier._parserFor so the import wizard actually invokes MacDiveXmlParser when the format detector recognises MacDive native XML. Test confirms detection of MacDive XML format advances the wizard to sourceConfirmation step.
The previously committed test stopped at format detection and didn't verify _parserFor actually returns MacDiveXmlParser. Added a second test that directly parses the fixture with MacDiveXmlParser and asserts on payload contents (specifically the 2 tags the fixture contains, which PlaceholderParser could not produce). Regression-safe for the switch case at line 429 of universal_import_providers.dart. Verified: test fails with PlaceholderParser, passes with MacDiveXmlParser.
Runs the user's 30MB MacDive XML export through MacDiveXmlParser and asserts: 540 dives, tag preservation (key M2 deliverable), site dedup, at least one dive with tanks + profile, and a depth p95 sanity check to catch converter regressions. Gated behind @tags(['real-data']) so CI stays fast.
- Align MacDive tank map with UddfEntityImporter: `volume` / `workingPressure` keys and `GasMix` object (importer does `t['gasMix'] as GasMix?`, so the prior Map would throw a TypeError and `volumeL` / `workingPressureBar` were silently dropped). - Return UTC wall-time from `MacDiveXmlReader._parseDate` so dive timestamps don't drift with device timezone / DST, matching the Subsurface parser convention. - Relax MacDive format detection to match `<dives` / `<schema` as prefixes so root tags with attributes / namespaces still detect. - Skip empty `<item/>` gear elements instead of producing a phantom `"||"` equipment entity. - Use `buddyRefs` (resolved via `buddyIdMapping`) instead of `unmatchedBuddyNames` so deselected buddies don't get created unconditionally — mirrors SubsurfaceXmlParser.
- MacDiveXmlReader now rejects non-`<dives>` root elements with a clear FormatException, instead of silently producing an empty logbook when a user forces the MacDive XML source onto a UDDF or other file. The parser's try/catch converts the exception into a user-visible ImportWarning. - MacDive XML real-sample test replaces the hardcoded `/Users/ericgriffin/...` path with a `MACDIVE_XML_SAMPLE` compile- time env var and skip-safe fallback, mirroring the UDDF sample test. CI and fresh clones now stay green. - MacDive XML parser links imported gear back onto each dive via `equipmentRefs` keyed by the composite `(manufacturer|name|serial)` uddfId, so dives actually reference the equipment entities created from their `<gear>` children. Added tests covering both the normal case and the empty-item path (no phantom refs).
Addresses the PR #254 round-3 review: - `dive_tags` has no UNIQUE(diveId, tagId) constraint, so duplicate `<tag>` entries on one dive would insert duplicate junction rows (tag shown twice on the imported dive). Dedup `tagRefs` before emitting on the dive map. - `addBuddyToDive` already upserts, so duplicate `<buddy>` entries won't create junk rows, but the redundant DB round-trips are wasted work. Apply the same list-contains dedup used for `equipmentRefs`. - Use the existing `MacDiveValueMapper.rating` helper instead of re-implementing clamp/round inline, keeping the rating rules consistent with the planned MacDive SQLite importer. Adds two regression tests covering duplicate `<buddy>` and `<tag>` entries in a single dive.
ericgriffin
force-pushed
the
feature/macdive-native-xml
branch
from
af121c3 to
28227e3
Compare
ericgriffin
added a commit
that referenced
this pull request
Apr 23, 2026
- Align MacDive tank map with UddfEntityImporter: `volume` / `workingPressure` keys and `GasMix` object (importer does `t['gasMix'] as GasMix?`, so the prior Map would throw a TypeError and `volumeL` / `workingPressureBar` were silently dropped). - Return UTC wall-time from `MacDiveXmlReader._parseDate` so dive timestamps don't drift with device timezone / DST, matching the Subsurface parser convention. - Relax MacDive format detection to match `<dives` / `<schema` as prefixes so root tags with attributes / namespaces still detect. - Skip empty `<item/>` gear elements instead of producing a phantom `"||"` equipment entity. - Use `buddyRefs` (resolved via `buddyIdMapping`) instead of `unmatchedBuddyNames` so deselected buddies don't get created unconditionally — mirrors SubsurfaceXmlParser.
ericgriffin
added a commit
that referenced
this pull request
Apr 23, 2026
- MacDiveXmlReader now rejects non-`<dives>` root elements with a clear FormatException, instead of silently producing an empty logbook when a user forces the MacDive XML source onto a UDDF or other file. The parser's try/catch converts the exception into a user-visible ImportWarning. - MacDive XML real-sample test replaces the hardcoded `/Users/ericgriffin/...` path with a `MACDIVE_XML_SAMPLE` compile- time env var and skip-safe fallback, mirroring the UDDF sample test. CI and fresh clones now stay green. - MacDive XML parser links imported gear back onto each dive via `equipmentRefs` keyed by the composite `(manufacturer|name|serial)` uddfId, so dives actually reference the equipment entities created from their `<gear>` children. Added tests covering both the normal case and the empty-item path (no phantom refs).
github-project-automation
Bot
moved this from In review
to Done
in Submersion Release Tracker
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Second of four milestones addressing MacDive import issues. Closes the
"XML format not recognised" complaint from ScubaBoard thread 667061
(glazerama, post #296) and the tag-import gap — MacDive UDDF doesn't emit
tags but MacDive XML does, so this is the path users should choose if
they want tags preserved.
Stacked on PR #252 (M1). Merge #252 first, then this.
What landed
ImportFormat.macdiveXmlwithMacDive (XML)source override.<dives>+<schema>fallback.MacDiveXmlReader: XML → typedMacDiveXmlLogbook, SI canonical unitsat the boundary via
MacDiveUnitConverter(feet→m, °F→°C, psi→bar,lb→kg, cft-at-psi→L water capacity).
MacDiveValueMapper:waterType/entryType/ratingwith tolerantcase-insensitive / substring matching. Shared with M3 (MacDive SQLite).
MacDiveXmlParser: implementsImportParser, maps into unifiedImportPayloadwith per-dive inline dedup for sites, buddies, tags, gear._parserForswitch inUniversalImportNotifier.Test plan
What's still deferred
Builds on the shared value mapper and unit converter from this PR.
Related