Repository navigation
Conversation
Add Move Goal, Exercise Goal, Stand Hours and Stand Goal sensors, read from the day's HKActivitySummary via HKActivitySummaryQuery. Move and Exercise progress stay the existing sample-based Active Energy and Exercise Time sensors; the goal IDs extend theirs. A ring HealthKit isn't tracking comes back with a goal of zero: Exercise and Stand without an Apple Watch, and the Move ring while set to move time (checked via activityMoveMode). Those sensors read unavailable rather than reporting a zero that was never measured. Zero progress towards a set goal is reported as zero. The activity summary type can't be observed or background-delivered, so Stand Hours observes the stand hour sample type instead and requests it alongside the summary type. Goals observe nothing: changing a goal writes no sample, so they refresh on periodic updates. Fix the update signaler to compare the set of enabled metrics rather than their HealthKit type identifiers: metrics sharing one identifier could otherwise never add their own observed types. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Hi @chewrocca
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🟡 Changes recommended
A new source file violates the repository’s one-top-level-type-per-file convention.
1 open finding
What changed in this PR
Adds opt-in Apple Health activity-ring goal and stand-hour sensors backed by daily HKActivitySummary data.
Changes:
- Adds ring models, catalog entries, querying, permissions, and background observation.
- Adds localized availability guidance.
- Adds comprehensive catalog, mapping, and sensor tests.
| File | Description |
|---|---|
Tests/Shared/Sensors/HealthKitSensor.test.swift |
Tests observation updates and whole-number states. |
Tests/Shared/Sensors/HealthKitMetricCatalog.test.swift |
Validates ring catalog metadata and footers. |
Tests/Shared/Sensors/HealthKitActivityRing.test.swift |
Tests summary mapping, permissions, and calendar-day handling. |
Sources/Shared/Resources/Swiftgen/Strings.swift |
Adds generated footer accessors. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitService.swift |
Queries summaries and configures read/observable types. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitSensorUpdateSignaler.swift |
Tracks enabled metrics by unique ID. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitSensor.swift |
Handles activity-summary query windows. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitMetricFooter.swift |
Adds ring availability footer types. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitMetricAggregation.swift |
Adds activity-summary aggregation. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitMetric+Activity.swift |
Defines the four new sensors. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitMetric.swift |
Documents summary-backed metrics. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitActivityRing+HealthKit.swift |
Maps summaries and observer sample types. |
Sources/Shared/API/Webhook/Sensors/Health/HealthKitActivityRing.swift |
Defines ring and summary-field models. |
Sources/App/Resources/en.lproj/Localizable.strings |
Adds English ring footer text. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Move HealthKitActivitySummaryField and its HealthKit extension into files of their own, matching the one-type-per-file layout of the neighbouring HealthKit metric files. No behaviour change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (78.78%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6044 +/- ##
==========================================
+ Coverage 54.81% 56.08% +1.27%
==========================================
Files 1248 1259 +11
Lines 82776 83382 +606
==========================================
+ Hits 45370 46765 +1395
+ Misses 37406 36617 -789
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thanks for this, it's nicely scoped and the tests are thorough. The signaler fix is a good catch too.
Since three of the four sensors depend on a Watch, could you test on an iPhone paired with an Apple Watch before we merge? I'd especially like to see how the goals behave around midnight (see inline).
| func goal(in summary: HKActivitySummary) -> HKQuantity { | ||
| switch self { | ||
| case .move: return summary.activeEnergyBurnedGoal | ||
| case .exercise: return summary.appleExerciseTimeGoal |
There was a problem hiding this comment.
appleExerciseTimeGoal and appleStandHoursGoal are deprecated in favor of exerciseTimeGoal and standHoursGoal (iOS 16+, and we target 16.4). The new ones came with watchOS 9, when these goals became adjustable, so they're the ones to trust for a custom goal. They're also optional, which gives us a cleaner "not tracked" signal than a goal of 0. Could you switch to them and update the test fixture?
There was a problem hiding this comment.
Switched to exerciseTimeGoal / standHoursGoal in c7d2677 and updated the test fixture. Thanks, I'd missed that those were deprecated.
| if ring == .move, summary.activityMoveMode != .activeEnergy { | ||
| return false | ||
| } | ||
| return goal.doubleValue(for: unit) > 0 |
There was a problem hiding this comment.
With the optional goals, nil can mean "not tracked" for Exercise and Stand here, and the > 0 check only needs to stay for Move.
There was a problem hiding this comment.
Done. nil is now the signal for Exercise and Stand. Move keeps the move-mode and > 0 checks since its energy goal is always present.
| if let error { | ||
| continuation.resume(throwing: error) | ||
| } else { | ||
| continuation.resume(returning: summaries?.first) |
There was a problem hiding this comment.
HKActivitySummaryQuery only returns days that already have a summary. Right after midnight, before the Watch syncs anything, I think all goal sensors would go unavailable and then come back, leaving a gap in their history every day. Can you confirm on a paired device? If it does happen, falling back to the most recent summary for goals would avoid it.
There was a problem hiding this comment.
I'll confirm this on a paired device as part of the test I describe below, but rather than leave the gap open until then I implemented the fallback you suggested: goals now read the latest summary within their 7-day lookback window, while progress still reads today's only. If a paired Watch turns out to pre-create the day's summary, the fallback is harmless. A test covers the two windows.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Please also create a documentation PR over this one home-assistant/companion.home-assistant#1409 |
Read exerciseTimeGoal and standHoursGoal instead of their deprecated apple-prefixed predecessors. They're optional, so a missing goal is the "not tracked" signal for Exercise and Stand; only Move, whose energy goal is always present, still checks the move mode and a goal above 0. A goal now reads the latest activity summary in its lookback window rather than today's only, so it no longer drops to unavailable between midnight and the day's first sample. Progress still reads today's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Thanks for the review. All three inline points are addressed in c7d2677, and the docs PR is up at home-assistant/companion.home-assistant#1449, stacked on #1409. On device testing: I don't currently have an Apple developer setup, so I haven't been able to run this on a phone yet. I'll get one set up, install the build on an iPhone with a paired Watch, and monitor the four sensors for a day or two, with particular attention to what happens around midnight. I'll report back here with what I see. On the Codecov patch check: the uncovered lines are the HealthKit query calls themselves, which can't execute in unit tests, same as the existing code in that file. Everything around them is covered. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Instead of this repeated subtitle, can we have a badge "Requires Apple Watch"? same UI as the badge that exists now that says "When app is open" for sensors that only update when app is open |
Replace the footer repeated under Exercise Goal, Stand Hours and Stand Goal with a "Requires Apple Watch" badge in the same style as the "When app is open" badge. That badge becomes SensorNoteBadge, one capsule view with a preset for each fact, and HealthKitMetric gains a requiresAppleWatch flag the health rows read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Done in bd27659. The three Watch-only rows now carry a "Requires Apple Watch" badge instead of the footer. I made the existing "When app is open" badge a shared |
bgoncal
left a comment
There was a problem hiding this comment.
Whenever changing the UI please make snapshot tests
Cover the row with a value and switched off, the Requires Apple Watch badge on and off, the two footers, and both presets of the shared note badge, in light and dark. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Please, rewrite the PR body following the PR template, link the documentation PR, I will then review the docs PR and we can merge |
|
ha. fair. Added in e5b6425: Recorded snapshot of the badge rows: And the same rows on the phone, from today's build: |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
bgoncal
left a comment
There was a problem hiding this comment.
Please, rewrite the PR body following the PR template, link the documentation PR, I will then review the docs PR and we can merge
|
ha. fair. PR body rewritten on the template, with the light and dark snapshots under Screenshots and the docs PR linked: home-assistant/companion.home-assistant#1449 (stacked on #1409). |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
AI is not allowed in PR communication, just a heads up. |











AI Policy
Select exactly one option that describes AI usage in this contribution:
Summary
Adds four opt-in Apple Health sensors read from the day's
HKActivitySummary, the object behind the Fitness app's rings, so a Home Assistant dashboard can draw the rings with the user's real goals instead of guessed ones. Move and Exercise progress already exist as the sample-based Active Energy and Exercise Time sensors; this adds what those can't provide: the daily goals, and stand hours, which have no quantity type to sum.unique_idhealth_active_energy_burned_goalhealth_apple_exercise_time_goalhealth_apple_stand_hourshealth_apple_stand_hours_goalEach goal's ID extends the ID of the sensor it is the goal for, and shares its unit and device class. They sit in the Activity section of Settings > Sensors > Apple Health Sensors, start off like every other Apple Health sensor, are read-only, and are read only during normal sensor updates. Existing sensors are untouched. Like the sleep stages and VO2 Max already in the catalog, three of them depend on an Apple Watch; without one they read
unavailable, and their rows carry a "Requires Apple Watch" badge.Closes the request in https://github.com/orgs/home-assistant/discussions/5132.
How it works:
.activitySummary(field), besidecumulativeSum,mostRecentandsleep.HealthKitActivityRingandHealthKitActivitySummaryFieldare plain data like the rest of the catalog; the HealthKit mapping lives in their+HealthKitfiles, following the sleep metrics' layout.exerciseTimeGoalandstandHoursGoalare optional). Its sensors read asunavailablerather than a zero that was never measured. The Move goal additionally checksactivityMoveMode, so a Move ring set to move time is never reported as kilocalories.unavailablebetween midnight and the day's first sample. Stand Hours reads today's only.SensorNoteBadge, one capsule view with a preset per fact, reused for "Requires Apple Watch".Tested on an iPhone 16 Pro Max with a paired Apple Watch: all four values match the Fitness app in the app and in Home Assistant, and the goals held through midnight with no gap (details and history graphs in the comments below).
Screenshots
Recorded snapshots of the badge rows, light and dark:
The rows on a phone, from a device build:
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#1449
Any other notes