feat: import full Apple Health export.zip with metrics and routes - #42
Conversation
Sideloaded installs cannot use HealthKit directly. This adds a local import path for the complete Apple Health export archive, not just individual GPX files. - Parse export.xml Records (HRV, sleep, steps, calories, vitals, etc.) - Parse ActivitySummary daily ring data - Import running workouts with linked workout-routes GPX files - Persist daily summaries in Drift (native) and SharedPreferences (web) - Merge imported metrics with workout rollups in ImportedHealthRepository - Update Settings and dashboard CTAs for export.zip import Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR adds Apple Health export.zip import support, including ZIP parsing, daily summary aggregation and storage, summary merging, an import use case, and updated settings/dashboard UI for selecting and previewing export.zip or GPX files. ChangesApple Health Export Import Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant HealthImportPage
participant ImportAppleHealthExportUseCase
participant AppleHealthExportParser
participant ImportedHealthStore
participant ImportWorkoutUseCase
HealthImportPage->>ImportAppleHealthExportUseCase: call(zipBytes, now)
ImportAppleHealthExportUseCase->>AppleHealthExportParser: parseZip(zipBytes)
AppleHealthExportParser-->>ImportAppleHealthExportUseCase: summaries, workouts, recordCount
ImportAppleHealthExportUseCase->>ImportedHealthStore: saveSummaries(summaries)
loop each workout
ImportAppleHealthExportUseCase->>ImportWorkoutUseCase: call(workout, routePoints, now)
ImportWorkoutUseCase-->>ImportAppleHealthExportUseCase: result
end
ImportAppleHealthExportUseCase-->>HealthImportPage: ImportAppleHealthExportResult
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/settings/presentation/pages/health_import_page.dart (1)
1-281: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winFile likely exceeds the ~250-line guideline for hand-written Dart files.
With both the Apple Health export preview card and the GPX preview card now inline in
build(), this file has grown well past ~250 lines. Per coding guidelines, larger UI should be split intopresentation/widgets/(e.g. extract the two preview cards into their own widget files).As per coding guidelines, "Keep hand-written files under ~250 lines; split larger code into
presentation/widgets/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/settings/presentation/pages/health_import_page.dart` around lines 1 - 281, The HealthImportPage file is now too large because the Apple Health export preview card and GPX preview card are built inline in HealthImportPage.build. Extract those UI sections into separate reusable widgets under presentation/widgets/, then compose them back in _HealthImportPageState.build to keep the hand-written file under the size guideline.Source: Coding guidelines
🧹 Nitpick comments (5)
test/infrastructure/health/import/apple_health_export_parser_test.dart (1)
9-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider covering non-ASCII content to catch encoding regressions.
The fixture only contains ASCII characters, and content is added via
.codeUnits(matching the parser'sString.fromCharCodesapproach). This means the UTF-8 decoding issue flagged inapple_health_export_parser.dartwouldn't be caught by this suite. Once fixed to useutf8.decode, consider adding a non-ASCIIsourceName/metadata value encoded withutf8.encodeto lock in correct behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infrastructure/health/import/apple_health_export_parser_test.dart` around lines 9 - 34, The Apple Health export parser tests currently only exercise ASCII content and build archive files with codeUnits, so they won’t catch UTF-8 decoding regressions. Update the apple_health_export_parser_test setup to include at least one non-ASCII metadata value such as sourceName in the exported XML, and encode that fixture content with utf8.encode when creating the ArchiveFile entries. Keep the existing parser-related test flow intact so the test verifies the utf8.decode behavior in apple_health_export_parser.dart.lib/infrastructure/health/import/apple_health_export_parser.dart (1)
1-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFile exceeds the ~250-line guideline.
This file is 293 lines. Consider extracting
_WorkoutBuilder(and its attribute-reading helper, which duplicates_attr) into its own file to bringapple_health_export_parser.dartunder the repository's size guideline.As per coding guidelines, "Keep hand-written files under ~250 lines; split larger code into
presentation/widgets/."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/infrastructure/health/import/apple_health_export_parser.dart` around lines 1 - 293, The parser file is over the repository size guideline and should be split up. Move `_WorkoutBuilder` and its attribute-reading helper logic out of `AppleHealthExportParser` into a separate file, and reuse the existing `_attr`-style parsing instead of duplicating it. Keep `AppleHealthExportParser`, `_parseExportXml`, and `_finalizeWorkout` focused on archive parsing so the main file drops under the line limit.Source: Coding guidelines
test/infrastructure/health/import/apple_health_record_aggregator_test.dart (1)
6-46: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test combining
Record+ActivitySummaryfor the same metric.This suite doesn't exercise a day with both an
ActiveEnergyBurned/AppleExerciseTimeRecordand a matchingActivitySummary, which is the scenario that surfaces the double-counting issue flagged inapple_health_record_aggregator.dart. Adding such a case would catch the regression once the aggregation logic is corrected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infrastructure/health/import/apple_health_record_aggregator_test.dart` around lines 6 - 46, Add a test in AppleHealthRecordAggregator coverage that uses both addRecord and addActivitySummary for the same day and same metrics, since the current test only verifies activity data through addActivitySummary. In apple_health_record_aggregator.dart, the relevant behavior is around AppleHealthRecordAggregator.finalize and the handling of active energy/exercise data, so create a case with an ActiveEnergyBurned/AppleExerciseTime Record plus a matching ActivitySummary and assert the day is not double-counted. Keep the existing aggregate-style test structure and target the same AppleHealthRecordAggregator API so the regression is caught reliably.lib/domain/usecases/health/import_apple_health_export_usecase.dart (1)
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDomain use case imports infrastructure directly.
AppleHealthExportParserandImportedHealthStoreare infrastructure types, imported directly here rather than through ashared/providers/DI boundary. This mirrors the pre-existing pattern inImportWorkoutUseCase, so it's not new debt, but it continues the violation as the import surface grows.As per coding guidelines, "Follow Clean Architecture: domain depends on infrastructure only through shared/providers/ as the dependency-injection boundary."
Also applies to: 24-35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/health/import_apple_health_export_usecase.dart` around lines 3 - 4, The `ImportAppleHealthExportUseCase` is importing infrastructure types directly, which bypasses the DI boundary. Update the use case to depend on providers from `shared/providers/` instead of `AppleHealthExportParser` and `ImportedHealthStore`, and wire those dependencies through the existing provider layer the same way other clean-architecture use cases should. Keep the use case focused on orchestration by referencing the provider-backed abstractions rather than importing infrastructure classes directly.Source: Coding guidelines
test/domain/usecases/health/import_apple_health_export_usecase_test.dart (1)
11-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for failure paths.
Only the happy path is tested. Consider adding cases for a malformed zip/xml (
FormatException→HealthDataFailure) and a store failure (→StorageFailure), since malformed user-provided exports are a realistic scenario for this feature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/domain/usecases/health/import_apple_health_export_usecase_test.dart` around lines 11 - 66, Add negative-path coverage to ImportAppleHealthExportUseCase in the existing test group: the current test only verifies the successful import flow. Add one test that feeds malformed zip or XML input through the use case and asserts the resulting failure is mapped from FormatException to HealthDataFailure, and another test that forces the store path in DriftImportedHealthStore to fail and asserts the use case returns StorageFailure. Use the existing ImportAppleHealthExportUseCase, ImportWorkoutUseCase, and DriftImportedHealthStore setup so the new cases exercise the same import pipeline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/domain/usecases/health/import_apple_health_export_usecase.dart`:
- Around line 42-43: The Apple Health import flow is doing synchronous ZIP
parsing on the main thread, which can block the UI during large imports. Update
`ImportAppleHealthExportUsecase` so `parseZip()` runs behind an async background
boundary using `compute()` or `Isolate.run`, and keep the `saveSummaries` call
after the background parse completes. Also check the settings import page path
that invokes the same parser directly and route it through the same off-thread
parsing approach.
In `@lib/features/settings/presentation/pages/health_import_page.dart`:
- Around line 29-59: Move the Apple Health zip parsing out of the UI thread:
`_pickFile()` currently calls `AppleHealthExportParser.parseZip()` synchronously
after `FilePicker.platform.pickFiles`, and
`ImportAppleHealthExportUseCase.call()` also parses `export.zip` on the main
isolate. Update `AppleHealthExportParser.parseZip()` usage so the heavy
archive/XML decoding runs in a background isolate, and avoid `withData: true` in
`_pickFile()` unless the full bytes are truly required. Keep the existing flow
in `_pickFile()`, `ImportAppleHealthExportUseCase`, and
`AppleHealthExportParser`, but restructure them so large exports don’t block the
UI or duplicate in-memory file copies.
- Around line 60-64: The GPX import preview currently decodes raw bytes with
String.fromCharCodes(bytes), which can corrupt non-ASCII metadata; update the
health import flow to decode the bytes as UTF-8 before passing content to
GpxWorkoutParser.parse in HealthImportPage. Also, HealthImportPage is too large,
so move the preview-related UI into a separate widget under
presentation/widgets/ and keep the page focused on orchestration while
preserving the existing _gpxPreview handling.
In `@lib/infrastructure/health/import/apple_health_export_parser.dart`:
- Around line 46-58: The Apple Health import still loads the entire ZIP into
memory because parseZip() decodes from List<int> bytes while the picker already
holds the raw archive data. Update the import flow around
AppleHealthExportParser.parseZip and the FilePicker-based entry point to read
the ZIP from the file stream/path instead of withData:true plus _zipBytes, and
have the parser consume a stream-backed archive source so the raw bytes are not
retained. Keep the export.xml lookup and _parseExportXml flow unchanged, but
rework the archive loading path to avoid double-buffering for large exports.
- Around line 74-82: The archive/XML decoding in Apple health import is using
byte-to-string conversion that can corrupt non-ASCII text; update the parsing
flow in AppleHealthExportParser, especially _findExportXml and the GPX/XML read
path, to decode file bytes with utf8.decode instead of String.fromCharCodes.
Make the same change at both call sites where export.xml and GPX content are
turned into strings so the parser preserves the original text before XML
parsing.
In `@lib/infrastructure/health/import/apple_health_record_aggregator.dart`:
- Around line 39-40: The daily active calories and exercise minutes are being
double-counted because `addRecord` and `addActivitySummary` both accumulate into
the same `acc.activeCalories` and `acc.exerciseMinutes` fields. Update
`AppleHealthRecordAggregator` so `ActivitySummary` is treated as the source of
truth for a day, and skip `Record`-based accumulation when a summary already
exists for that date (or otherwise ensure only one source contributes per day).
Use the `hasActivitySummary`/per-day aggregation logic in `addRecord` and
`addActivitySummary` to keep the totals from being summed twice.
---
Outside diff comments:
In `@lib/features/settings/presentation/pages/health_import_page.dart`:
- Around line 1-281: The HealthImportPage file is now too large because the
Apple Health export preview card and GPX preview card are built inline in
HealthImportPage.build. Extract those UI sections into separate reusable widgets
under presentation/widgets/, then compose them back in
_HealthImportPageState.build to keep the hand-written file under the size
guideline.
---
Nitpick comments:
In `@lib/domain/usecases/health/import_apple_health_export_usecase.dart`:
- Around line 3-4: The `ImportAppleHealthExportUseCase` is importing
infrastructure types directly, which bypasses the DI boundary. Update the use
case to depend on providers from `shared/providers/` instead of
`AppleHealthExportParser` and `ImportedHealthStore`, and wire those dependencies
through the existing provider layer the same way other clean-architecture use
cases should. Keep the use case focused on orchestration by referencing the
provider-backed abstractions rather than importing infrastructure classes
directly.
In `@lib/infrastructure/health/import/apple_health_export_parser.dart`:
- Around line 1-293: The parser file is over the repository size guideline and
should be split up. Move `_WorkoutBuilder` and its attribute-reading helper
logic out of `AppleHealthExportParser` into a separate file, and reuse the
existing `_attr`-style parsing instead of duplicating it. Keep
`AppleHealthExportParser`, `_parseExportXml`, and `_finalizeWorkout` focused on
archive parsing so the main file drops under the line limit.
In `@test/domain/usecases/health/import_apple_health_export_usecase_test.dart`:
- Around line 11-66: Add negative-path coverage to
ImportAppleHealthExportUseCase in the existing test group: the current test only
verifies the successful import flow. Add one test that feeds malformed zip or
XML input through the use case and asserts the resulting failure is mapped from
FormatException to HealthDataFailure, and another test that forces the store
path in DriftImportedHealthStore to fail and asserts the use case returns
StorageFailure. Use the existing ImportAppleHealthExportUseCase,
ImportWorkoutUseCase, and DriftImportedHealthStore setup so the new cases
exercise the same import pipeline.
In `@test/infrastructure/health/import/apple_health_export_parser_test.dart`:
- Around line 9-34: The Apple Health export parser tests currently only exercise
ASCII content and build archive files with codeUnits, so they won’t catch UTF-8
decoding regressions. Update the apple_health_export_parser_test setup to
include at least one non-ASCII metadata value such as sourceName in the exported
XML, and encode that fixture content with utf8.encode when creating the
ArchiveFile entries. Keep the existing parser-related test flow intact so the
test verifies the utf8.decode behavior in apple_health_export_parser.dart.
In `@test/infrastructure/health/import/apple_health_record_aggregator_test.dart`:
- Around line 6-46: Add a test in AppleHealthRecordAggregator coverage that uses
both addRecord and addActivitySummary for the same day and same metrics, since
the current test only verifies activity data through addActivitySummary. In
apple_health_record_aggregator.dart, the relevant behavior is around
AppleHealthRecordAggregator.finalize and the handling of active energy/exercise
data, so create a case with an ActiveEnergyBurned/AppleExerciseTime Record plus
a matching ActivitySummary and assert the day is not double-counted. Keep the
existing aggregate-style test structure and target the same
AppleHealthRecordAggregator API so the regression is caught reliably.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 755900d8-21c2-4a85-ae08-33e18b8a93e8
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
lib/domain/usecases/health/import_apple_health_export_usecase.dartlib/features/dashboard/presentation/pages/run_history_page.dartlib/features/dashboard/presentation/widgets/connect_healthkit_card.dartlib/features/settings/presentation/pages/health_import_page.dartlib/features/settings/presentation/pages/settings_page.dartlib/infrastructure/health/drift_imported_health_store.dartlib/infrastructure/health/health_infrastructure_providers.dartlib/infrastructure/health/import/apple_health_date_parser.dartlib/infrastructure/health/import/apple_health_export_parser.dartlib/infrastructure/health/import/apple_health_record_aggregator.dartlib/infrastructure/health/import/apple_health_unit_converter.dartlib/infrastructure/health/imported_health_database.dartlib/infrastructure/health/imported_health_database.g.dartlib/infrastructure/health/imported_health_repository.dartlib/infrastructure/health/imported_health_store.dartlib/infrastructure/health/imported_summary_merger.dartlib/infrastructure/health/prefs_imported_health_store.dartpubspec.yamltest/domain/usecases/health/import_apple_health_export_usecase_test.darttest/features/settings/health_import_page_test.darttest/fixtures/apple_health_export.xmltest/infrastructure/health/import/apple_health_export_parser_test.darttest/infrastructure/health/import/apple_health_record_aggregator_test.dart
| AppleHealthExportParseResult parseZip(List<int> zipBytes) { | ||
| final archive = ZipDecoder().decodeBytes(zipBytes); | ||
| final files = _indexArchive(archive); | ||
|
|
||
| final exportXml = _findExportXml(files); | ||
| if (exportXml == null) { | ||
| throw const FormatException( | ||
| 'Could not find export.xml inside the archive', | ||
| ); | ||
| } | ||
|
|
||
| return _parseExportXml(exportXml, files); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does archive package ZipDecoder decodeBytes decompress all file contents eagerly into memory, or lazily on content access, in version 4.0.9?
💡 Result:
In version 4.0.9 of the archive package, the ZipDecoder.decodeBytes method eagerly processes the provided byte buffer [1][2]. When you use ZipDecoder.decodeBytes(bytes), the entire input byte array is loaded into memory [1][3]. Furthermore, accessing the content of individual files within the resulting Archive object—typically by referencing the ArchiveFile.content property—often results in that content being decompressed and stored in memory [4][5]. If you do not explicitly clear this data (e.g., using file.clear), the archive object will hold onto the decompressed bytes, which can lead to high memory usage or Out of Memory (OOM) errors when handling large archives [4][5]. To avoid loading all file contents into memory, the library provides the decodeStream method instead of decodeBytes [1][2]. By using an InputFileStream, you can decode the archive without storing the entire archive or its decompressed contents in memory [1][6]. For even greater efficiency, you can use entity.writeContent(outputStream) to decompress file contents directly to disk, bypassing memory entirely for the extracted data [1][6].
Citations:
- 1: https://pub.dev/documentation/archive/latest/
- 2: https://chromium.googlesource.com/external/github.com/brendan-duncan/archive/+/f01d6a340ffe24e0ef46fa682d1b6bcc7b7aef13
- 3: https://github.com/brendan-duncan/archive
- 4: Out of Memory brendan-duncan/archive#251
- 5: [BUG] HUGE performance tank after 3.1.7 update brendan-duncan/archive#183
- 6: https://github.com/brendan-duncan/archive/blob/main/README.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline lib/infrastructure/health/import/apple_health_export_parser.dart --view expanded || true
printf '\n== relevant file excerpt ==\n'
cat -n lib/infrastructure/health/import/apple_health_export_parser.dart | sed -n '1,220p'
printf '\n== parseZip call sites ==\n'
rg -n "parseZip\(" lib || true
printf '\n== export zip / Apple Health import references ==\n'
rg -n "export\.zip|Apple Health|apple_health|health export|ZipDecoder|decodeStream|decodeBytes" lib || trueRepository: YKDBontekoe/KYNOS
Length of output: 13403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== import page excerpt ==\n'
cat -n lib/features/settings/presentation/pages/health_import_page.dart | sed -n '1,260p'
printf '\n== usecase excerpt ==\n'
cat -n lib/domain/usecases/health/import_apple_health_export_usecase.dart | sed -n '1,220p'
printf '\n== any file-picker / bytes loading references ==\n'
rg -n "readAsBytes|bytes\b|pickFiles|XFile|FilePicker|file_picker|readAsBytesSync|getData" lib/features/settings/presentation/pages/health_import_page.dart lib/domain/usecases/health/import_apple_health_export_usecase.dart lib -g '!**/*.g.dart' || trueRepository: YKDBontekoe/KYNOS
Length of output: 18121
Avoid loading the Apple Health export twice in memory. FilePicker.platform.pickFiles(withData: true) plus _zipBytes already keeps the full ZIP resident, so switching only ZipDecoder().decodeBytes(zipBytes) won’t materially reduce peak usage. For large exports, the import flow needs to stream/read from file and avoid retaining the raw archive bytes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/infrastructure/health/import/apple_health_export_parser.dart` around
lines 46 - 58, The Apple Health import still loads the entire ZIP into memory
because parseZip() decodes from List<int> bytes while the picker already holds
the raw archive data. Update the import flow around
AppleHealthExportParser.parseZip and the FilePicker-based entry point to read
the ZIP from the file stream/path instead of withData:true plus _zipBytes, and
have the parser consume a stream-backed archive source so the raw bytes are not
retained. Keep the export.xml lookup and _parseExportXml flow unchanged, but
rework the archive loading path to avoid double-buffering for large exports.
- Parse export.zip in a background isolate via Isolate.run - Read picked files from path on native (withData only on web) - Decode XML/GPX as UTF-8 instead of String.fromCharCodes - Prefer ActivitySummary over duplicate Record rollups per day - Extract preview cards into settings presentation widgets - Add negative-path and UTF-8 regression tests Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/infrastructure/health/import/apple_health_export_parser.dart (1)
171-190: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch the XML parser errors here too
XmlDocument.parsecan throwXmlException-based errors on malformed GPX, which bypass thisFormatExceptionhandler and abort the zip import. Catch the XML exception type as well so a bad route just gets skipped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/infrastructure/health/import/apple_health_export_parser.dart` around lines 171 - 190, The GPX route parsing loop in apple_health_export_parser.dart only skips FormatException, so malformed XML from _gpxParser.parse can still abort the import. Update the try/catch around the route parsing in the builder.routePaths loop to also catch the XmlException-based parser errors thrown by XmlDocument.parse, and treat them the same as a bad route by continuing to the next path.
♻️ Duplicate comments (1)
lib/infrastructure/health/import/apple_health_export_parser.dart (1)
48-70: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftZip is still fully materialized in memory.
ZipDecoder().decodeBytes(zipBytes)plus eagerly copying every file's.contentinto_indexArchive's map (even unused workout-route GPX files) keeps large Apple Health exports fully resident in memory. This was flagged in a prior review and remains unresolved in this diff.For very large
export.ziparchives, consider usingarchive'sdecodeStream/InputFileStream(native) or lazily readingfile.contentonly forexport.xmland the specific route paths actually referenced by valid running workouts, rather than eagerly indexing every entry's decompressed bytes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/infrastructure/health/import/apple_health_export_parser.dart` around lines 48 - 70, The zip handling in AppleHealthExportParser still eagerly loads the entire Apple Health export into memory, which leaves large archives resident unnecessarily. Update parseZip/_indexArchive to avoid ZipDecoder().decodeBytes(zipBytes) plus copying every file’s content up front; instead use archive’s stream-based decoding/InputFileStream, and only read export.xml and any route GPX files that are actually referenced by valid workouts through the relevant parser methods.
🧹 Nitpick comments (2)
test/domain/usecases/health/import_apple_health_export_usecase_test.dart (1)
94-127: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFake store implemented by hand rather than
mocktail.Repo guidelines call for
mocktailfakes for repository contract tests; this test hand-rolls_FailingImportedHealthStore. Not blocking, but consider switching to amocktail-based mock for consistency.As per coding guidelines, "Repository contracts must be tested with
mocktailfakes, and widget tests must useProviderScopewith overridden fakes rather than real repositories."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/domain/usecases/health/import_apple_health_export_usecase_test.dart` around lines 94 - 127, The test currently uses a hand-rolled _FailingImportedHealthStore instead of the repo-standard mocktail fake for a repository contract test. Replace this custom implementation with a mocktail-based mock/fake in import_apple_health_export_usecase_test.dart, and configure saveSummaries to throw the disk-full error there so the test stays aligned with the repository contract pattern and existing mocking conventions.Source: Coding guidelines
lib/features/settings/presentation/widgets/gpx_import_preview_card.dart (1)
63-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a shared formatter for preview timestamps
_formatDateis duplicated inlib/features/settings/presentation/pages/manual_run_page.dartandlib/features/dashboard/presentation/pages/run_route_page.dart, and the duration formatting follows the same pattern as other run views. Extracting the shared formatting would keep these previews consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/settings/presentation/widgets/gpx_import_preview_card.dart` around lines 63 - 72, The preview timestamp and duration formatting in the GPX import card is duplicated across other run views, so centralize it instead of keeping a local implementation. Move the shared date/time and duration formatting logic out of the widget’s private helpers (_formatDate and _formatDuration) into a common formatter used by the related run pages, then update the GPX preview card to call that shared formatter so all previews stay consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/domain/usecases/health/import_apple_health_export_usecase.dart`:
- Line 3: `ImportAppleHealthExportUsecase` is depending on
`apple_health_export_isolate.dart` directly from `infrastructure`, which
violates the repo’s Clean Architecture boundary. Remove the direct
infrastructure import from `import_apple_health_export_usecase.dart` and inject
the needed dependency through the `shared/providers/` DI layer instead. Update
the usecase to depend on an abstraction or provider-managed instance, and wire
the concrete `apple_health_export_isolate` implementation only in the provider
setup.
---
Outside diff comments:
In `@lib/infrastructure/health/import/apple_health_export_parser.dart`:
- Around line 171-190: The GPX route parsing loop in
apple_health_export_parser.dart only skips FormatException, so malformed XML
from _gpxParser.parse can still abort the import. Update the try/catch around
the route parsing in the builder.routePaths loop to also catch the
XmlException-based parser errors thrown by XmlDocument.parse, and treat them the
same as a bad route by continuing to the next path.
---
Duplicate comments:
In `@lib/infrastructure/health/import/apple_health_export_parser.dart`:
- Around line 48-70: The zip handling in AppleHealthExportParser still eagerly
loads the entire Apple Health export into memory, which leaves large archives
resident unnecessarily. Update parseZip/_indexArchive to avoid
ZipDecoder().decodeBytes(zipBytes) plus copying every file’s content up front;
instead use archive’s stream-based decoding/InputFileStream, and only read
export.xml and any route GPX files that are actually referenced by valid
workouts through the relevant parser methods.
---
Nitpick comments:
In `@lib/features/settings/presentation/widgets/gpx_import_preview_card.dart`:
- Around line 63-72: The preview timestamp and duration formatting in the GPX
import card is duplicated across other run views, so centralize it instead of
keeping a local implementation. Move the shared date/time and duration
formatting logic out of the widget’s private helpers (_formatDate and
_formatDuration) into a common formatter used by the related run pages, then
update the GPX preview card to call that shared formatter so all previews stay
consistent.
In `@test/domain/usecases/health/import_apple_health_export_usecase_test.dart`:
- Around line 94-127: The test currently uses a hand-rolled
_FailingImportedHealthStore instead of the repo-standard mocktail fake for a
repository contract test. Replace this custom implementation with a
mocktail-based mock/fake in import_apple_health_export_usecase_test.dart, and
configure saveSummaries to throw the disk-full error there so the test stays
aligned with the repository contract pattern and existing mocking conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 16d74e7e-a7df-4a34-b9e3-64ae2db47ccb
📒 Files selected for processing (16)
lib/domain/usecases/health/import_apple_health_export_usecase.dartlib/features/settings/presentation/pages/health_import_page.dartlib/features/settings/presentation/widgets/apple_health_export_preview_card.dartlib/features/settings/presentation/widgets/gpx_import_preview_card.dartlib/features/settings/presentation/widgets/health_import_preview_row.dartlib/infrastructure/health/import/apple_health_export_isolate.dartlib/infrastructure/health/import/apple_health_export_parser.dartlib/infrastructure/health/import/apple_health_record_aggregator.dartlib/infrastructure/health/import/apple_health_workout_builder.dartlib/shared/utils/picked_file_bytes.dartlib/shared/utils/picked_file_bytes_io.dartlib/shared/utils/picked_file_bytes_web.darttest/domain/usecases/health/import_apple_health_export_usecase_test.darttest/fixtures/apple_health_export.xmltest/infrastructure/health/import/apple_health_export_parser_test.darttest/infrastructure/health/import/apple_health_record_aggregator_test.dart
✅ Files skipped from review due to trivial changes (2)
- lib/shared/utils/picked_file_bytes.dart
- test/fixtures/apple_health_export.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/infrastructure/health/import/apple_health_record_aggregator.dart
- lib/features/settings/presentation/pages/health_import_page.dart
| @@ -0,0 +1,82 @@ | |||
| import 'package:kynos/core/errors/failures.dart'; | |||
| import 'package:kynos/domain/usecases/health/import_workout_usecase.dart'; | |||
| import 'package:kynos/infrastructure/health/import/apple_health_export_isolate.dart'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Domain usecase importing infrastructure/ directly.
lib/domain/usecases/health/import_apple_health_export_usecase.dart imports apple_health_export_isolate.dart from infrastructure/. Per Clean Architecture rules for this repo, domain should reach infrastructure only through the shared/providers/ DI boundary, not via direct imports.
As per coding guidelines, "Follow Clean Architecture: domain depends on infrastructure only through shared/providers/ as the dependency-injection boundary."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/health/import_apple_health_export_usecase.dart` at line
3, `ImportAppleHealthExportUsecase` is depending on
`apple_health_export_isolate.dart` directly from `infrastructure`, which
violates the repo’s Clean Architecture boundary. Remove the direct
infrastructure import from `import_apple_health_export_usecase.dart` and inject
the needed dependency through the `shared/providers/` DI layer instead. Update
the usecase to depend on an abstraction or provider-managed instance, and wire
the concrete `apple_health_export_isolate` implementation only in the provider
setup.
Source: Coding guidelines
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
2 participants
Summary
Adds support for importing the complete Apple Health
export.ziparchive locally, so sideloaded installs can load health data without HealthKit API access.Previously, only single GPX files could be imported. This change parses the full Apple export structure:
export.xmlhealth records — steps, HRV, resting HR, heart rate, SpO₂, respiratory rate, sleep, active/basal calories, distance, flights climbed, exercise minutes, running power/cadence/strideActivitySummaryelements — daily activity ring rollups (active calories, exercise time)Workoutelements — running workouts with linkedworkout-routes/*.gpxGPS routes from the zipArchitecture
AppleHealthExportParser— decodes zip, streamsexport.xml, resolves GPX route filesAppleHealthRecordAggregator— maps Apple HK record types toHealthSummary(mirrors HealthKit aggregator)ImportAppleHealthExportUseCase— persists daily summaries + workouts via existing import storeImportedDailySummariesDrift table (schema v2) + SharedPreferences on webImportedHealthRepositorymerges stored summaries with workout-derived rollupsUI
.zipor.gpxexport.zipValidation
flutter analyze— clean (3 minor test warnings only)flutter test— 68 tests passingflutter build web— succeedsSummary by CodeRabbit
New Features
export.zipalongside GPX.Bug Fixes
Chores