Repository navigation
fix(ios): export chart HR sources and refresh write authorization - #2645
Merged
Merged
Conversation
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.
What this PR does
Apple Health could receive NOOP sleep and nightly vitals while missing continuous heart rate even when Today plotted it. The HR writer read only
noopDeviceId, whereas the charts resolve the active/registered/canonical strap union. Read the same repository union at 60-second resolution so re-paired and registered strap data reaches the writer with the charts' source precedence.Track the actual authorization sets with sorted
read:/write:identifiers. Adding write access for an already-readable type now changes the fingerprint, and sleep categories and workout routes are included. The existingwriteDeniedfilter remains authoritative. Retain the legacy preference key so an old read-only fingerprint triggers a foreground refresh; save the new fingerprint only after a successful request.This is iOS-specific. Android's Health Connect HR writer intentionally uses the registry's active strap id (#814); no analytics, database schema, or stored physiological values change. This patch addresses the two root causes in #2644. Status diagnostics, newest-first backfill, and nightly-vital timestamps remain outside its scope. The open HealthKit work in #2642, #2298, and #959 was checked; those cover different behavior, though #959 will need to reconcile this fingerprint with its staged consent flow if it lands later.
Type of change
How it was tested
Swift 6.3.3
swiftc -frontend -parse StrandiOS/Health/HealthKitBridge.swiftpassed on Linux. This is syntax parsing, not an Apple SDK type check.A temporary standalone Swift harness compiled the production fingerprint getter and both repository HR-bucket methods against small type/store stand-ins. Assertions passed for stable ordering, write-only expansion, read/write role separation, category/route expansion and removal, legacy fingerprint mismatch; registered-only HR, active precedence, canonical fallback, ascending buckets, 60-second window forwarding, and empty/error store handling. The harness does not execute HealthKit authorization or saves.
Tools/core Python suite: 153 tests passed. Parity repository-acceptance suite: 50 tests passed.python3 Tools/doc_comment_lint.py, a focused lint scan of the changed iOS file,python3 Tools/i18n_audit.py --ci origin/main, andgit diff --checkpassed.Upstream CI passed all five checks on head
71297b59840a4929e20704f3efc581c6c56f8dd7:NOOPiOSsimulator build, universal macOS build plusStrandTests, Tools Python CI, source hygiene, and i18n. Apple build/test run.Draft validation limit: no real-device HealthKit authorization sheet or save was exercised. An on-device upgrade authorization / re-paired HR export check is still required; no successful HealthKit export or downstream Bevel recovery result is claimed.
Checklist
docs/CONTRIBUTING.mdRelated issues
Refs #2644