fix(healthkit): scope background observer updates - #2233
Merged
Merged
Conversation
Preserve delivered HealthKit types and advance native anchors only after server writes succeed to prevent all-type replays and callback expiry. Apply UUID deletion tombstones before committing each anchor.
…32766197-v1 # Conflicts: # docs/production-incident-baseline.md
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughHealthKit observer synchronization now scopes updates by type, uses native-managed anchored queries with explicit completion, uploads deletion tombstones through a new server mutation, and narrows background observer registration. Tests and incident documentation cover the updated flows. ChangesHealthKit synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HealthKitObserver
participant BackgroundHealthKitSync
participant AppleHealthSyncService
participant HealthKitSyncRouter
HealthKitObserver->>BackgroundHealthKitSync: deliver typed observer update
BackgroundHealthKitSync->>AppleHealthSyncService: syncObserverChanges(typeIdentifiers)
AppleHealthSyncService->>HealthKitSyncRouter: push samples and delete UUIDs
HealthKitSyncRouter-->>AppleHealthSyncService: return sync counts
AppleHealthSyncService->>HealthKitObserver: complete observer and anchored query
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 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 |
Contributor
Reviewer's GuideScopes background HealthKit background observer sync to delivered sample types, introduces anchored quantity queries with two-phase commit semantics and deletion tombstones, and wires end-to-end support across mobile, native Swift, and server plus regression tests and documentation. Sequence diagram for anchored HealthKit quantity sync with two-phase commitsequenceDiagram
participant JS as syncHealthKitObserverChanges
participant HK as HealthKitAdapter
participant API as healthKitSync.deleteQuantitySamples
participant API2 as healthKitSync.pushQuantitySamples
JS->>HK: queryAnchoredSamples(typeIdentifier, initialStartDate)
HK-->>JS: { queryId, samples, deletedUUIDs }
JS->>API2: mutate({ samples })
API2-->>JS: { inserted, errors }
alt [upload succeeded and deletedUUIDs not empty]
loop batched up to 500
JS->>API: mutate({ deletedUUIDs, typeIdentifier })
API-->>JS: { deleted }
end
end
alt [all uploads succeeded and queryId]
JS->>HK: completeAnchoredQuery(typeIdentifier, queryId, true)
HK-->>JS: true
else [upload failed and queryId]
JS->>HK: completeAnchoredQuery(typeIdentifier, queryId, false)
HK-->>JS: true
end
Sequence diagram for scoped background HealthKit observer syncsequenceDiagram
participant Obs as HKObserverQuery
participant Bg as background-health-kit-sync
participant Sync as AppleHealthSyncService
participant JS as syncHealthKitObserverChanges
Obs-->>Bg: addSampleUpdateListener({ typeIdentifier, updateId })
Bg->>Bg: pendingUpdates.set(updateId, typeIdentifier)
Bg->>Bg: drainSyncQueue()
alt [catch-up sync]
Bg->>Sync: syncObserverChanges({ typeIdentifiers: BACKGROUND_HEALTH_KIT_TYPES })
else [type-scoped sync]
Bg->>Sync: syncObserverChanges({ typeIdentifiers: unique(pendingUpdates.values) })
end
Sync->>JS: syncObserverChanges({ typeIdentifiers })
JS-->>Sync: { inserted, errors }
Sync-->>Bg: result
Bg->>Obs: completeObserverUpdates(updateIds, succeeded)
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
PR Summary by Qodofix(healthkit): scope background observer sync to delivered types
AI Description
Diagram
High-Level Assessment
Files changed (25)
|
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
Code Review by Qodo
Context used✅ Compliance rules (platform):
192 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Assert deletion branch boundaries and side-effect payloads so Stryker detects regressions in tombstone scoping, cache invalidation, and telemetry.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
…32766197-v1 # Conflicts: # docs/production-incident-baseline.md
Move deletion persistence into the repository with typed results, preserve actionable API failures, and reject partial observer syncs so HealthKit retries safely.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
…32766197-v1 # Conflicts: # docs/production-incident-baseline.md
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
…32766197-v1 # Conflicts: # docs/production-incident-baseline.md
|
LGTM! The changes cleanly implement two-phase anchored HealthKit synchronization and fix the previous feedback:
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
Asherlc
enabled auto-merge (squash)
July 29, 2026 01:26
PR Review SummaryOverall, this is a clean and robust implementation of HealthKit two-phase anchored query synchronization and background observer change processing. Key observations:
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mobile/modules/health-kit/Tests/HealthKitAnchorStoreTests.swift (1)
24-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for
unknownQuery/mismatchedTypecompletion errors.The new
HealthKitAnchoredQueryCoordinatorError.unknownQueryand.mismatchedTypecases (inHealthKitAnchorStore.swift) guard the two-phase commit against stale/cross-typequeryIdmisuse, but no test exercises either path here.🧪 Suggested additional tests
func testCompleteWithUnknownQueryIdThrows() async throws { let coordinator = HealthKitAnchoredQueryCoordinator( anchorStore: HealthKitAnchorStore(userDefaults: defaults) ) XCTAssertThrowsError( try coordinator.complete(typeIdentifier: "heart-rate", queryId: "not-a-real-id", succeeded: true) ) { error in guard case HealthKitAnchoredQueryCoordinatorError.unknownQuery = error else { XCTFail("Unexpected error: \(error)") return } } } func testCompleteWithMismatchedTypeIdentifierThrows() async throws { let coordinator = HealthKitAnchoredQueryCoordinator( anchorStore: HealthKitAnchorStore(userDefaults: defaults) ) let result = try await coordinator.run(typeIdentifier: "heart-rate") { _ in return ((), HKQueryAnchor(fromValue: 1)) } XCTAssertThrowsError( try coordinator.complete( typeIdentifier: "step-count", queryId: try XCTUnwrap(result.queryId), succeeded: true ) ) { error in guard case HealthKitAnchoredQueryCoordinatorError.mismatchedType = error else { XCTFail("Unexpected error: \(error)") return } } }🤖 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 `@packages/mobile/modules/health-kit/Tests/HealthKitAnchorStoreTests.swift` around lines 24 - 123, Add tests alongside the existing coordinator completion tests for both error paths: verify completing with an unregistered query ID throws HealthKitAnchoredQueryCoordinatorError.unknownQuery, and verify completing a valid query ID with a different type identifier throws .mismatchedType. Use HealthKitAnchoredQueryCoordinator.run and complete, asserting the specific enum cases.
🤖 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 `@packages/mobile/app/_layout.cleanup.test.tsx`:
- Line 196: Update the deleteQuantitySamples mock used by the cleanup tests to
use mockResolvedValue with a result object containing the required deleted
number, matching the mutation contract and the other mocks in this block.
In `@packages/mobile/lib/background-health-kit-sync.test.ts`:
- Around line 678-683: Update the deferred result type in the firstSync setup to
use the real queryAnchoredSamples shapes: import HealthKitSample directly from
the health-kit module and type samples as HealthKitSample[] and deletedUUIDs as
string[]. Preserve the existing queryId field and mockQueryAnchoredSamples
behavior.
In `@packages/mobile/lib/background-health-kit-sync.ts`:
- Around line 76-79: Update the result annotation in the background sync flow to
use Awaited<ReturnType<AppleHealthSyncService["syncObserverChanges"]>>, matching
the method invoked in the try block. Leave the syncObserverChanges call and
surrounding error handling unchanged.
In `@packages/mobile/lib/health-kit-sync.test.ts`:
- Around line 585-593: Update the health-kit sync test around the deletion
mutation and healthKit.completeAnchoredQuery assertions to verify invocation
order, not only call arguments: assert the deleteQuantitySamples mutation occurs
before completeAnchoredQuery succeeds. Preserve the existing argument assertions
while adding an invocation-order check using the recorded mock call order.
In `@packages/mobile/lib/health-kit-sync.ts`:
- Around line 484-512: The observer route-sync loop should batch routes into a
single mutation instead of calling pushWorkoutRoutes once per workout. Update
the surrounding syncHealthKitToServer flow to collect each non-empty workout
route with its workoutUuid and sourceName, then invoke
trpcClient.healthKitSync.pushWorkoutRoutes.mutate once after collection, while
preserving database-inaccessible rethrows, exception capture, and per-workout
error reporting.
- Around line 442-449: Update the sync completion flow around
completeAnchoredQuery to await its boolean result, fail the operation when the
native call returns false, and only acknowledge the observer after a successful
anchor commit. Correct the returned SyncResult so inserted contains only
uploadResult.inserted; add and propagate an optional deleted field for the
deleted count through background-health-kit-sync.ts, or remove the count if it
is not needed.
In `@packages/mobile/test-setup.ts`:
- Around line 486-497: The anchored-query mock shape is duplicated across three
Health Kit mock factories. In packages/mobile/test-setup.ts lines 486-497,
create and export a colocated createHealthKitAnchorMocks factory or shared
default result object, typing the anchored result from queryAnchoredSamples’
return type; update packages/mobile/app/providers/[id].test.tsx lines 319-327
and packages/mobile/lib/useAutoSync.test.ts lines 52-61 to spread that shared
factory instead of redeclaring queryId, samples, and deletedUUIDs.
In `@packages/server/src/repositories/health-kit-sync-repository.test.ts`:
- Around line 1157-1166: Strengthen the assertion in the “rejects an invalid
typed deletion result” test for
HealthKitSyncRepository.processDeletedQuantitySamples by matching the specific
schema-validation error, following the neighbouring test’s toBeInstanceOf
pattern. Keep the existing invalid mocked result and method arguments, but
ensure the test fails if rejection occurs for an unrelated reason.
In `@packages/server/src/repositories/health-kit-sync-repository.ts`:
- Around line 406-419: Update the deletion flow to capture the rows returned by
executeWithSchema in the repository method containing this SQL, and return the
number of returned records rather than uniqueUUIDs.length. Preserve the existing
delete criteria and return shape while ensuring zero and partial matches report
their actual deleted counts.
- Around line 381-404: Update the deletion handling block for body measurements
and metric streams to invoke replaceRows through the publisher instance, using
publisher.replaceRows within the uniqueUUIDs Promise.all flow; remove the
extracted unbound replaceRows reference while preserving the existing
availability check and arguments.
- Around line 371-405: Update processDeletedQuantitySamples to detect
additiveDailyMetric and pointInTimeDailyMetric types before the health_event
deletion fallback, then reaggregate the affected fitness.daily_metrics rows
using the existing daily-metric logic keyed by date, provider_id, and
source_name. Preserve the current UUID-based canonical-store deletion behavior
for body measurements, metric streams, and other non-daily quantities.
In `@packages/server/src/routers/health-kit-sync.test.ts`:
- Around line 165-168: Update the deleteQuantitySamples input validation to
enforce UUID-formatted deletion identifiers with Zod’s z.uuid(), then replace
the arbitrary deletedUUIDs test fixtures at all referenced cases with valid UUID
values while preserving the existing tombstone-publishing assertions.
- Around line 245-267: Update the production error path used by
deleteQuantitySamples to report unexpected repository failures through Sentry
before returning INTERNAL_SERVER_ERROR. Pass repositoryError to captureException
with the endpoint and user context, then update the test assertion for
mockSentryCaptureException to verify those arguments instead of expecting no
call.
---
Outside diff comments:
In `@packages/mobile/modules/health-kit/Tests/HealthKitAnchorStoreTests.swift`:
- Around line 24-123: Add tests alongside the existing coordinator completion
tests for both error paths: verify completing with an unregistered query ID
throws HealthKitAnchoredQueryCoordinatorError.unknownQuery, and verify
completing a valid query ID with a different type identifier throws
.mismatchedType. Use HealthKitAnchoredQueryCoordinator.run and complete,
asserting the specific enum cases.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e40c6dfc-889a-4f83-835d-5171487f54b2
📒 Files selected for processing (25)
docs/production-incident-baseline.mdpackages/mobile/app/_layout.cleanup.test.tsxpackages/mobile/app/_layout.tsxpackages/mobile/app/providers/[id].test.tsxpackages/mobile/app/providers/index.test.tsxpackages/mobile/lib/apple-health-provider.test.tspackages/mobile/lib/apple-health-provider.tspackages/mobile/lib/background-health-kit-sync.test.tspackages/mobile/lib/background-health-kit-sync.tspackages/mobile/lib/health-kit-sync.test.tspackages/mobile/lib/health-kit-sync.tspackages/mobile/lib/useAutoSync.test.tspackages/mobile/modules/health-kit/README.mdpackages/mobile/modules/health-kit/Tests/HealthKitAnchorStoreTests.swiftpackages/mobile/modules/health-kit/Tests/HealthKitTypesTests.swiftpackages/mobile/modules/health-kit/index.test.tspackages/mobile/modules/health-kit/index.tspackages/mobile/modules/health-kit/ios/HealthKitAnchorStore.swiftpackages/mobile/modules/health-kit/ios/HealthKitModule.swiftpackages/mobile/modules/health-kit/ios/HealthKitTypes.swiftpackages/mobile/test-setup.tspackages/server/src/repositories/health-kit-sync-repository.test.tspackages/server/src/repositories/health-kit-sync-repository.tspackages/server/src/routers/health-kit-sync.test.tspackages/server/src/routers/health-kit-sync.ts
Keep anchor commits and deletion telemetry truthful across native, mobile, and server boundaries.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Owner
Author
|
Addressed the PR-level Swift coverage request in commit 41cbe75. HealthKitAnchorStoreTests now covers unknown query completion and verifies that a type mismatch preserves the pending anchor for a later correctly typed completion. The full Swift package suite passes: 77 tests. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
This was referenced Jul 31, 2026
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
Scopes background HealthKit sync to delivered types and registers only sample types consumed by the pipeline.
Uses two-phase native anchors, server-side UUID tombstones, and 500-item deletion batches so failed uploads retry safely.
Adds Swift, mobile, and server regression coverage plus incident and runbook documentation.
Validation
pnpm lint, root/server/web TypeScript checks, 14,207 unit/mobile tests, 75 Swift tests, and the ad-hoc signed Release simulator build pass.Summary by Sourcery
Scope HealthKit background observer sync to delivered sample types and introduce anchor-aware, type-specific incremental syncing with safe retry semantics for quantity deletions.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by cubic
Scopes HealthKit background observer sync to only the delivered types and commits native anchors only after server writes succeed, keeping callbacks fast and preventing all-type replays. Adds provider‑scoped UUID tombstones and strict failure handling so deletions and retries are safe.
Bug Fixes
typeIdentifier; coalesce by type and runsyncHealthKitObserverChanges(...).queryIdand requires aninitialStartDate; commit viacompleteAnchoredQuery(type, queryId, succeeded)only after server success.healthKitSync.deleteQuantitySamples; apply provider‑scoped UUID tombstones, batch up to 500, invalidate user cache, and return actionable errors (PRECONDITION_FAILED,HealthKitDeletionTombstonesUnsupportedError).onSyncComplete, acknowledge viacompleteObserverUpdates([...], false), and capture warnings in Sentry.cypressbinary downloads duringpnpm install; document image scan timeout.Migration
queryAnchoredSamples(type, initialStartDate)/completeAnchoredQuery(...)and background type scoping).Written for commit 7fd7bbb. Summary will update on new commits.
Summary by CodeRabbit