Skip to content

GroveHealthKitFHIR rework - #221

Merged
lukaskollmer merged 116 commits into
mainfrom
lukas/grove-fhir-api-changes
Oct 30, 2026
Merged

lukaskollmer merged 116 commits into
mainfrom
lukas/grove-fhir-api-changes

Conversation

@lukaskollmer

@lukaskollmer lukaskollmer commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

♻️ Current situation & Problem

reworked and updated MHC to use the new Grove FHIR APIs introduced in SchmiedmayerLab/Grove#118

⚙️ Release Notes

📚 Documentation

yes

✅ Testing

yes

Code of Conduct & Contributing Guidelines

By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

lukaskollmer and others added 30 commits August 29, 2026 23:12
Spezi was renamed to Grove in 0.3.0 (SchmiedmayerLab/Grove, PR #43): every
product `Spezi*` -> `Grove*` and `XCTSpezi*` -> `XCTGrove*`, the core type
`Spezi` -> `Grove`, and no source-compatibility typealiases. This moves the
app onto 0.3.0-beta.6.

Package graph:
- Both manifests and the Xcode project point at SchmiedmayerLab/Grove.git.
  The old URL still resolves through GitHub's rename redirect, but SwiftPM
  derives identity from the last path component, so a graph mixing the two
  URLs resolves one repository under two identities -- they have to move
  together.
- MyHeartCounts-StudyDefinitions is pinned to a revision until its own Grove
  release is tagged (SchmiedmayerLab/MyHeartCounts-StudyDefinitions#46);
  every 0.2.x of it still requires the `spezi` identity.
- The watch target's GroveHealthKit product dependency pointed at a package
  reference for the retired StanfordSpezi/SpeziHealthKit that was not in the
  project's packageReferences at all. It now resolves against the Grove
  reference, and the orphan is gone.

Renames the compiler cannot do for you, because the replacement is not the
`Spezi` token: `.spezi(_:)` -> `.grove(_:)`, `@Application(\.spezi)` ->
`\.grove`, `SpeziAppDelegate.spezi` -> `GroveAppDelegate.grove`, and
`UTType.speziStudyBundle` -> `UTType.studyBundle` (the vendor prefix is
dropped, not swapped).

Identifiers that name stored or transmitted data:
- BGTaskSchedulerPermittedIdentifiers now lists
  `$(PRODUCT_BUNDLE_IDENTIFIER).scheduler.refresh`. Grove honours the old
  string for one release and then stops; registering an unpermitted
  identifier fails silently, so this cannot be left to drift.
- The scheduler notification userInfo is read through
  `SchedulerNotifications.taskId(fromUserInfo:)`, which accepts the current
  key and the pre-Grove one. A direct subscript still compiles and would
  silently stop matching notifications already pending on a device.
- MHC's own coding system moved off the framework's name to an authority we
  own: `https://myheartcounts.stanford.edu/fhir/CodeSystem/sampleType`.
  Reads accept the superseded `https://spezi.stanford.edu` spelling, so
  historical bloodLipids / nicotineExposure / dietMEPAScore /
  mentalWellbeingScore observations keep parsing.
- MHC's hand-rolled sourceRevision writer derives its urls from Grove's
  `FHIRExtensionURL.sourceRevision` instead of hardcoding bdh.stanford.edu,
  so resources this app writes agree with the ones Grove writes.
  The backend still emits the old spellings: MyHeartCounts-Firebase#213.

Left alone deliberately: the `mhc.spezi.stanford.edu` config host, the
"based on the Stanford Spezi Template Application" provenance headers, and
the App Store description copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0182tQC5aCA5KdUYixuB7tvP
The `.bundledWithApp` fallback only caught `unableToDecode`, so a fetch
failure -- an offline launch, or a bucket that does not yet carry the
object under its current name -- threw `unableToFetchFromServer` straight
out of `_update(using:)`. That is not a soft landing: `update()`'s
`returnCachedBundleOnError` can only return a bundle already loaded in this
process, which on a cold launch is nothing, so the study bundle simply
fails to load.

Both failures now take the same path, still only for the firebase selector:
an explicit `--studyBundle <url>` that cannot be fetched keeps erroring
rather than quietly loading something else.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0182tQC5aCA5KdUYixuB7tvP
Grove deprecates `RWLock` (pthread_rwlock is slower than the alternatives in
nearly every access pattern, and it cannot own the state it guards), which
left the app with two deprecation warnings.

`MyHeartCountsStandard.currentEnrollmentInfo` becomes a
`Mutex<CurrentEnrollmentInfo?>`, so the lock owns the value instead of
guarding a `nonisolated(unsafe) static var` beside it, and the enrollment
snapshot is built before taking the lock rather than reading MainActor state
inside it. `Lifecycle` guards a keypath write rather than a value, so it
takes a `Mutex<Void>` -- the same swap Grove made internally for
`LocalPreference`.

Behaviourally equivalent: no reader relied on read-lock concurrency, and
both values were only ever read and written whole.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0182tQC5aCA5KdUYixuB7tvP
# Conflicts:
#	MyHeartCounts.xcodeproj/project.pbxproj
#	MyHeartCounts.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthDashboard+BloodPressure.swift
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthDashboard+SleepAnalysis.swift
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthDashboard.swift
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthDashboardQuantityLabel.swift
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthDashboardTile.swift
#	MyHeartCounts/Heart Health Dashboard/Health Dashboard/HealthStatsChart.swift
#	MyHeartCounts/Heart Health Dashboard/QuantitySample+FHIR.swift
#	MyHeartCounts/Heart Health Dashboard/QuantitySamplesQuery.swift
#	MyHeartCounts/Heart Health Dashboard/SleepSessionsQuery.swift
#	MyHeartCounts/Modules/Lifecycle.swift
#	MyHeartCounts/Modules/MHCBackgroundTasks.swift
#	MyHeartCounts/Modules/ManagedFileUpload.swift
#	MyHeartCounts/Modules/SetupTestEnvironment.swift
#	MyHeartCounts/MyHeartCountsDelegate.swift
#	MyHeartCounts/Onboarding/OnboardingDisclaimerStep.swift
#	MyHeartCounts/Root View/RootView.swift
#	MyHeartCounts/Supporting Files/Info.plist
#	MyHeartCounts/Utils/HealthKit.swift
#	MyHeartCountsShared/Package.swift
#	MyHeartCountsTests/ManagedFileUploadTests.swift
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 27422218-097a-4900-ab93-0125951be744
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

lukaskollmer and others added 15 commits October 7, 2026 13:41
HealthObservationBatch is gone. Converting is a static requirement,
HealthObservation.export(_:at:using:exporter:receive:): HealthKit samples go
through the Grove exporter, observations the app models itself become their
own resources and never ask for an exporter. uploadHealthObservations and
HealthUploadStaging are generic over the observation type and dispatch
statically, so nothing is cast per element. Self-modelled observations build
their issued-at FHIR instant once per batch, since creating FHIR date/time
primitives from a Date was measured to be expensive.

HealthUploadBatch keeps only the upload file format: the convert-and-write
overload is gone and the file-name rule moved in as HealthUploadBatch.filename.
Grove refusals of HealthKit samples log as errors. Tests follow the new API.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FHIRExchangeLedger holds the app's one HealthKit exporter, fenced to its
account generation. Getting it is a lock and a comparison; it throws
healthKitExporterUnavailable while none is built and staleAccountGeneration
for a batch captured before a logout. MyHeartCountsStandard rebuilds it in
refreshHealthKitExporter() at launch, sign-in and after enrollment changes,
and clears it on logout, so the identity secret is read and the producer set
up only when the account or the enrollment changes, not per batch.

Live staging, retractions and the storage upload path look the exporter up;
the conversion closure is synchronous, and self-modelled observations still
never ask for one. Retractions run on the exporter
(healthKitRetractions(of:recordedAt:)). The historical uploader still builds
its exporter per batch. Tests follow the new API; tests keep building their
exporter from the staging overrides.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rter With the Historical Upload

HealthKitFHIRExporter.RecordProvider replaces HealthObservationRecords: one
parameter instead of a HealthKit and a records source. .healthKit(HealthKit)
fetches an ECG's voltages and correlated symptoms and states its voltage count
for the batch split; _custom exists only for the batch-export tests.
HealthObservation.export takes the provider, and the exporter as an
autoclosure, so self-modelled observations never evaluate it.

HealthKitSamplesFHIRUploader holds its HealthKit and is created from the
Standard asynchronously. It now looks up the app's one HealthKit exporter, the
same instance live ingestion uses, instead of building one per batch; a batch
that runs before the launch refresh has built it refreshes once and retries.
This replaces the per-batch build that e2ffd4b still left for the historical
upload. Tests follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The ledger used a rollback journal (journal_mode = TRUNCATE) with synchronous
FULL and fullfsync, which Xcode flags as excessive I/O for a database not in
WAL mode. It now sets journal_mode = WAL itself in prepareDatabase, keeping
synchronous = FULL, fullfsync and adding checkpoint_fullfsync: every commit
still reaches storage before it returns, with fewer syncs per commit.

GRDB's journalMode = .wal is not used because it lowers synchronous to NORMAL
after prepareDatabase ran (seen in GRDB 7.11.1's Database.setUpWALMode(), not
documented), and a lost reservation could mint an event sequence again. An
existing database switches to WAL when next opened; the process-exclusive lock
guarantees no other connection holds it. The log files live in the ledger's
directory, which is already excluded from backup and deleted as a whole on a
discard. The durability test now pins wal and checkpoint_fullfsync.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the --measureExporterThroughput launch argument, the app's HealthKit
exporter measures its throughput (Grove's Options.measuresThroughput); every
export batch then logs the cumulative report, and the debug form's "HealthKit
Exporter Throughput" screen shows it live: totals and rates, each phase's
share, and each HealthKit type, with a button that copies the report. Live
ingestion and the historical upload share the one exporter, so the numbers
cover both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Grove's HealthKit, SensorKit and Questionnaire exporters now export and
retract asynchronously and build a call's graphs on a task group, on every
core by default. Their entry points are nonisolated(nonsending), so a call
runs on the caller's executor and the receive closures keep capturing local
state unchanged; MyHeartCounts keeps the default concurrency.

The batch export's flush, the Questionnaire observation extraction, the
SensorKit publication and the HealthKit retraction helper await the calls,
and their callers await them in turn. The test support's
withTemporaryDirectory takes an async body, and the concurrent ledger
contract test runs its nine workers on a task group, each returning its
failure count, instead of on DispatchQueue.concurrentPerform.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The two tests that corrupt the ledger's database file expected the open
connection to read the overwritten pages back, which bumping the file change
counter forced under the rollback journal. Since the ledger keeps a
write-ahead log, an open connection never reads that counter: it serves the
pages it cached, and the newest pages live in the log, so neither test saw an
error.

The helper now checkpoints the log into the database file, closes the
ledger's connection and then overwrites the ledgerEntry pages, so the
transaction after the reopen reads them and fails as corrupt. Both tests keep
their assertions; corruptPageDiscardsTheDatabase now states that the page is
corrupted while the ledger is closed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…codings

Each SensorKit upload strategy names its sidecar's file extension again where
it writes the bytes, as on main: PPG recordings are .mhcPPG2, tabular
recordings .csv, ECG and structured payloads .json. SensorKitUploadSidecar
carries that extension instead of a recording format whose computed extension
nothing tied to the bytes (Grove dropped it); PPG sidecars were .bin.

decode-ppg reads both PPG encodings through Grove: .mhcPPG files with Grove's
legacy decoder and .mhcPPG2 files with the registered one. The bytes carry no
version, so the extension decides; a .zstd or .zlib suffix on an uncompressed
earlier file is looked through. On 14 real .mhcPPG files the new command
yields the same samples as the previous one; on a .mhcPPG2 file the previous
command read every value byte-swapped. Dates are now ISO 8601 with the stored
instant's exact fractional seconds; .iso8601 dropped them, shifting every
sample's offset by up to a second.

MHCDataSpec describes both encodings. Building SensorKitCLI from
MyHeartCountsShared alone needs its Grove pin moved to a revision with the
legacy decoder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the Grove dependency from e1cbf2fd to b20cc933 (the head of Grove's
lukas/grove-fhir-api-changes) and MyHeartCounts-StudyDefinitions from 49289ddd
to aaa19dcf (its lukas/grove-fhir-api-changes, which pins the same Grove
revision), in the app project and in MyHeartCountsShared. In Package.resolved
only these two pins and the origin hash change; every other package stays at
its committed version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Grove reads legacy .mhcPPG files (SensorKitPPGRecording(legacyMyHeartCountsData:))
and writes the current format, and the SensorKit CLI decodes both through it,
so MyHeartCountsShared's BinaryEncoder/BinaryDecoder and PPGSample's binary
conformance had no caller left. They are removed with their tests, and so is
the swift-nio dependency only they used. Grove keeps a test that decodes bytes
this encoder wrote for the same three samples. PPGSample's JSON round trip,
which the CLI's output relies on, stays as PPGSampleCodingTests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves Grove from b20cc933 to eeab2ef5, the head of Grove's
lukas/grove-fhir-api-changes, and the study definitions from aaa19dc to
9346b78, which pins the same Grove revision. Grove now reports a workout
activity only as a shared activity it is a kind of, starts PPG
recordings with a version byte, keeps concurrent exports from waiting on
each other, and projects category, percent and rate observations back
to HealthKit, which questionnaire extraction uses.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Launched with --recordHealthKitExportLog, the app appends one JSON line
per HealthKit batch export to Library/Application Support/
HealthKitExportLog/export-log.jsonl: the batch's origin (live or
historical), samples by type, duration and outcome, with Grove's
refusals and warnings counted by kind or the error it failed with, then
the exporter's running throughput totals by phase and source type. Every
line names the app launch, device, OS and build it was written on, so a
test run's log can be copied off a connected device with devicectl and
analysed per run. The flag also turns on the exporter's throughput
measurement; without it nothing is written. A historical batch counts as
exported once its upload file is staged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A category's `Progress` and its pending files' size change without notifying SwiftUI, so the debug view showed
the queue as it was when opened. It now rereads the queue every two seconds and when pulled down, shows when it
last did, and states each category's completed and total upload counts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves Grove from eeab2ef5 to 46f5ef5f, the head of Grove's
lukas/grove-fhir-api-changes, and the study definitions from 9346b78 to
3a40a9c, which pins the same Grove revision. Grove now hashes every
attachment through one shared SHA-1, so its FHIR modules build on Linux,
and drains each built record's temporaries inside its concurrent build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukaskollmer
lukaskollmer marked this pull request as ready for review October 16, 2026 20:17
Moves Grove from 46f5ef5f to 6e40cfbf and the study definitions from
3a40a9c to fa1b3ee, which pins the same Grove revision. Both histories were
re-created with every commit signed; the code is identical, only the
commit identifiers changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukaskollmer
lukaskollmer force-pushed the lukas/grove-fhir-api-changes branch from 14e0166 to 3239341 Compare October 16, 2026 20:32
@lukaskollmer
lukaskollmer removed this pull request from stack #217 October 30, 2026 16:54
Base automatically changed from lukas/grove-fhir-review-fixes to main October 30, 2026 16:57
@lukaskollmer
lukaskollmer merged commit f8eb105 into main Oct 30, 2026
4 of 17 checks passed
@lukaskollmer
lukaskollmer deleted the lukas/grove-fhir-api-changes branch October 30, 2026 16:58
@github-project-automation github-project-automation Bot moved this from In Progress to Done in MyHeartCounts Oct 30, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in MyHeart Counts Project Planning Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants