Caffeine: import intakes from Apple Health (#949) - #985
Conversation
The caffeine window (#526) already stores exactly what a HealthKit caffeine sample carries — a timestamp and an optional mg — so a coffee logged in another app can feed the decay estimate instead of being typed in twice. A SAMPLE query, not the day-bucketed one water uses: the estimate decays each intake from its own time, so a day total would collapse a 7am coffee and a 9pm one into a number that answers no question the card asks. Dedup is harder than water's. Water is one day total that can simply be replaced; caffeine is a list of individual events. So an imported intake carries its HealthKit sample UUID in a new optional `externalId`, and a sync replaces the imported SUBSET wholesale, leaving hand-logged intakes alone. That keeps a re-import idempotent instead of logging a second copy of every coffee, and makes an intake deleted in the source app disappear here too. The field is optional so existing stored JSON still decodes, with every existing intake correctly reading back as hand-logged. Imported intakes are not editable here: remove/clearAll skip them and the UI offers no control, because the next sync would re-read the same window and bring the entry straight back. Remove it where it was logged. The store gains a `shared` instance — the card used to own its own, which was fine while every write came from the card, but two instances over one UserDefaults key would leave the card publishing a stale array while the imported intakes sat invisible on disk. Follows the LiquidPowerMonitor pattern already used for @mainactor ObservableObject singletons. ANDROID IS DELIBERATELY NOT MATCHED. Health Connect has no caffeine-only scope: caffeine is a field on NutritionRecord behind READ_NUTRITION, which grants the user's entire food log. Apple exposes .dietaryCaffeine as its own narrow type. Asking every Android user for their whole nutrition history to read one number is not a trade this app should make quietly, so the manifest is untouched and the gap is documented instead. Tests: 15 in StrandTests covering idempotency, the vanished-sample case, the backward-compatible decode, and the not-editable rules. No CI runs them — app-build.yml is disabled and StrandTests executes nowhere.
…ead error Two defects found re-reviewing the PR. The intake id was freshly minted on every sync. CaffeineIntake.id defaults to UUID(), and the bridge rebuilds every intake from the HealthKit samples each time, so the same coffee got a new identity on every pass. That defeated replaceImported's "nothing changed" guard (a pointless JSON rewrite and republish on every sync) and gave ForEach an entirely new set of rows to rebuild each time. The sample's own uuid is stable, so use it. My own test could not have caught this: it reused one CaffeineIntake value for both imports, so the ids matched trivially. It now builds a fresh intake per import, the way the bridge does, plus an explicit test that two intakes built from one sample are equal. Second, collectCaffeine discarded the query error, so a failed read returned an empty array and replaceImported deleted every imported intake — the same class of bug as the hydration one in the previous commit, and worse here because the caller's whole job is to replace the set. It now returns nil on failure, and only a genuine empty result clears anything.
The last instance of the pattern that has bitten four times today: the cast used `?? []`, so an unexpected sample type would read as "no caffeine" — and because the caller replaces the imported set wholesale, that would silently delete every imported intake. Vanishingly unlikely on a query typed to a quantity type, but it is the same failure shape as the error check sitting beside it, so it now shares that check.
|
Re-reviewed against current Still applies cleanly, and nothing has drifted under it. Both earlier re-review fixes are intact — the sample uuid as the intake identity, and One tightening, pushed. The remaining A dependency worth recording: this PR only works because #974 landed first. Adding The verification gap is the real story hereOnly two checks run on this PR:
That is not a caveat about this PR so much as a statement that this PR has no executable verification of any kind. Every claim in it rests on reading. Given today produced three self-inflicted defects in a PR I had already re-reviewed twice, I would not treat my own reading as sufficient here. Recommendation unchanged from when I opened it: this is the one that most wants a compile before merging — either a local |
1 participant
The caffeine half of #949. Replaces #975, which GitHub auto-closed when its base branch was deleted on merging #974 — same branch, same commits, now targeting
main.The caffeine window (#526) already stores exactly what a HealthKit caffeine sample carries — a timestamp and an optional mg — so a coffee logged elsewhere can feed the decay estimate instead of being typed in twice.
A sample query, not a day sum
Water uses
HKStatisticsCollectionQueryday buckets. Caffeine cannot: the estimate decays each intake from its own time, so a day total would collapse a 7am coffee and a 9pm one into a single number that answers no question the card asks.Dedup is harder here than for water
Water is one day total that can simply be replaced. Caffeine is a list of individual events. So an imported intake carries its HealthKit sample UUID in a new optional
externalId, and a sync replaces the imported subset wholesale, leaving hand-logged intakes untouched — giving the same two properties: re-importing is idempotent, and an intake deleted in the source app disappears here.externalIdis optional, so JSON written before this change still decodes, with every existing intake correctly reading back as hand-logged rather than as an import a later sync would feel entitled to delete. There is a test for exactly that.Imported intakes are not the user's to edit
removeandclearAllskip them and the UI offers no control, because the next sync re-reads the same window and would bring the entry back — offering the button would be offering something NOOP cannot honour. The row showsApple Healthinstead.One shared store
CaffeineLogStoregains asharedinstance and the card observes it rather than owning its own; two instances over one UserDefaults key would leave the card publishing a stale array while imported intakes sat invisible on disk. FollowsLiquidPowerMonitorexactly — the same@MainActor final class … ObservableObject+static let shared+@ObservedObjectshape already used across the Liquid views.Android is deliberately NOT matched
Health Connect has no caffeine-only scope: caffeine is a field on
NutritionRecordbehindREAD_NUTRITION, which grants the user's entire food log. Apple exposes.dietaryCaffeineas its own narrow type. Asking every Android user for their whole nutrition history to read one number is not a trade worth making quietly, so the manifest is untouched and the gap is documented. (The water half is matched — Health Connect has a narrowREAD_HYDRATION.)Two defects found re-reviewing this, both fixed
Imported intakes got a new identity every sync.
CaffeineIntake.iddefaults toUUID()and the bridge rebuilds each intake from the samples every pass, so the same coffee was a different row each time — defeatingreplaceImported's "nothing changed" guard (a pointless rewrite and republish per sync) and handingForEacha wholly new list. Now keyed on the sample uuid, which is stable. My own test could not have caught this: it reused a single value for both imports so the ids matched trivially. It now builds a fresh intake per import, plus an explicit equality test.A failed query deleted every imported intake.
collectCaffeinediscarded the error, so a failure returned an empty array andreplaceImportedcleared the set. It returns nil on failure now; only a genuine empty result clears anything.Verification — read this part
StrandTests/CaffeineImportTests.swift: idempotency, identity stability, the vanished-sample case, hand-logged intakes surviving a sync, the backward-compatible decode, the not-editable rules, and a persistence round-trip.app-build.ymlis disabled andStrandTestsexecutes nowhere. They are written and unexecuted — I am not claiming they pass, only that I could not run them.swiftc -parse. That is a syntax check, not a type-check; SwiftUI/HealthKit are unavailable on this Linux host.Honest summary: the logic is tested by construction and unexecuted, and the app target is uncompiled. Worth a local
xcodebuild(or a one-offapp-build.ymlrun) before merging — more so than #974, since there is no Android leg here to corroborate it.