Rather than create a new store object, use the app's shared HKHealthStore, loopDataManager.glucoseStore.healthStore (going through carbStore or doseStore would work just as well).

You can use .distantPast as the initial value here.

A style nit: if you invert the logic here, you can drop a level of indentation by moving the logic beneath the guard; i.e.

guard lastUpdateInterval >= .hours(24) else {
   return
}
// remainder of the function body

Since this function only performs work conditionally, consider naming it updateBedtimeIfNeeded.

Since we only update the bedtime date in the successful case, should this variable only be updated on that path?

This variable should be renamed, as it's intended to be the last time that we've attempted to find the bedtime. I had it update even in a failure case to ensure that where there's no sleep data, the query doesn't get attempted every time the function is called. Would this be the desired behavior?

That's reasonable—and it prompted me to look at the behavior of the bedtime computation function when no sleep data exists. Added an additional comment down there.

Each sample's value property will correspond to a value of HKCategoryValueSleepAnalysis. If a user has a sleep tracking app, they might have primarily .asleep values; if the data comes from Bedtime in the Clock app, they'll have primarily .inBed values. It's possible that a user has a value of each type each night, in which case they'll both be averaged.

Consider performing an additional step here which filters samples down to those with .inBed values to use in the computation. If there aren't "enough", try .asleep instead. In any case, we'll want to make sure we don't include any .awake values.

Done. For the purposes of "enough," what could be a decent threshold? The current code just needs one value for it to use the .inBed values instead of the .asleep values, but I'm not sure what the ideal amount would be

There's a tricky case here where the time might not exist within the next 24 hours, like in the hour jump that occurs for DST. It's worth doing a check that time.timeIntervalSinceNow <= .hours(24) before proceeding.

You might consider defining a SleepStoreError enum to use in SleepStoreResult, with a noMatchingBedtime case.

Since assertion failures trigger only in debug modes, it's best practice to still call the completion handler on this path—not that I'd expect it to happen!

This function's body does what you want: produce a scalar value that encodes the time of day to be averaged. However, this scalar isn't always the number of seconds from midnight; consider DST. Something like timeOfDayInSeconds would communicate the intention more accurately.

We'll want a guard to check that !samples.isEmpty to avoid trapping here if no sleep data is available. SleepStoreError can include a noSleepDataAvailable case to represent the failure.

You can use SleepStoreError as the type contained by the failure case.

It doesn't look like we'll be surfacing these errors to the user, so you can remove the localized descriptions.

sampleLimit strikes me more as a parameter to the query, rather than as a property of the SleepStore itself. Consider making this a parameter to getAverageSleepStartTime with a default value.

One detail here I forgot to mention—the time of day for a particular sleep sample is dependent upon the time zone in which the sample was recorded. For example, if a user went on vacation recently, using their home time zone to determine the time of day in seconds could yield a result that doesn't accurately reflect when they went to sleep in their vacation location's time zone.

Add a parameter to this function of type TimeZone, and assign it to the timeZone property of calendar. You can retrieve the TimeZone for each sleep sample by retrieving the name string from the sample's metadata via HKMetadataKeyTimeZone.

For unwrapping the timezone, what could an appropriate default be? We could use the current time zone, or exclude the sample from the analysis (though this would probably mean it wouldn't be possible to use a reduce function to average the samples)

I just checked my sleep data, from SleepTracker, and it has timezone. Whether or not this field is present will depend on the app, and since it's metadata, the app isn't forced to store it. I think assuming local timezone in that case is probably the best option, as the user probably will have no samples with timezone.

Sounds good. Just checked my sleep data (some from Bedtime, some from SleepCycle) and it all contains timezones

I believe we no longer need a HKHealthStore instance directly inside WatchDataManager, since the access is managed through SleepStore; is this property used anywhere?

Nope, removed

Here's another case where newBedtime could be more than a day in the future in case of DST. We could add a newBedtime.timeIntervalSince(bedtime) <= .hours(24) clause similarly to before; alternatively, see my comment in complicationUserInfoTransferInterval below.

We might defensively make the condition in this ternary also check an upper bound on timeUntilBedtime, i.e. (0..<TimeInterval(hours: 24)).contains(timeUntilBedtime), to ensure that complication transfers aren't slowed down if bedtime somehow ended up being a Date further in the future.

I just checked my sleep data, from SleepTracker, and it has timezone. Whether or not this field is present will depend on the app, and since it's metadata, the app isn't forced to store it. I think assuming local timezone in that case is probably the best option, as the user probably will have no samples with timezone.