From 4dc6c33b36d45d9b1bef834cf1306eb19df4d106 Mon Sep 17 00:00:00 2001 From: ryanbr Date: Thu, 30 Jul 2026 12:13:06 -0700 Subject: [PATCH 1/2] Caffeine: import intakes from Apple Health (#949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Strand/Data/CaffeineLog.swift | 51 ++++++- Strand/Screens/CaffeineLogCard.swift | 30 +++-- StrandTests/CaffeineImportTests.swift | 179 +++++++++++++++++++++++++ StrandiOS/Health/HealthKitBridge.swift | 52 ++++++- 4 files changed, 297 insertions(+), 15 deletions(-) create mode 100644 StrandTests/CaffeineImportTests.swift diff --git a/Strand/Data/CaffeineLog.swift b/Strand/Data/CaffeineLog.swift index 5526d2ac2c..efb4bb9e90 100644 --- a/Strand/Data/CaffeineLog.swift +++ b/Strand/Data/CaffeineLog.swift @@ -116,9 +116,19 @@ public struct CaffeineIntake: Codable, Equatable, Identifiable, Sendable { public let at: Date /// Amount in mg, if the user gave one. nil = "logged it, didn't say how much" — we never invent a number. public let mg: Double? + /// The Apple Health sample this came from, or nil when the user logged it here (#949). + /// + /// Optional so old stored JSON — which has no such field — still decodes, with every existing intake + /// correctly reading back as hand-logged. It carries the HealthKit sample UUID, which is what lets a + /// re-import recognise a drink it already knows rather than logging a second copy of it. + public let externalId: String? - public init(id: UUID = UUID(), at: Date, mg: Double? = nil) { - self.id = id; self.at = at; self.mg = mg + /// True when this came from Apple Health rather than from a tap in NOOP. Imported intakes are not + /// editable here — the app that logged the drink owns it. + public var isImported: Bool { externalId != nil } + + public init(id: UUID = UUID(), at: Date, mg: Double? = nil, externalId: String? = nil) { + self.id = id; self.at = at; self.mg = mg; self.externalId = externalId } } @@ -178,6 +188,14 @@ public struct CaffeineActiveEstimate: Equatable, Sendable { @MainActor public final class CaffeineLogStore: ObservableObject { + /// The process-wide store (#949). + /// + /// The card used to own its own instance, which was fine while every write came from the card + /// itself. The Apple Health import writes from outside it, and two instances over one UserDefaults + /// key would mean the card kept publishing its stale in-memory array until it was rebuilt — the + /// imported intakes would be on disk and invisible. One instance, one source of truth. + public static let shared = CaffeineLogStore() + /// Logged intakes, newest first. Persisted as JSON under one UserDefaults key. @Published public private(set) var intakes: [CaffeineIntake] { didSet { save() } } @@ -210,14 +228,39 @@ public final class CaffeineLogStore: ObservableObject { intakes = ([CaffeineIntake(at: date, mg: cleanMg)] + intakes).sorted { $0.at > $1.at } } + /// Replace every IMPORTED intake with `imported`, leaving hand-logged ones untouched (#949). + /// + /// Wholesale replacement rather than a merge, for the same reason the imported hydration row is + /// replaced rather than added to: the health store hands back the full window each time, so writing + /// it whole makes a re-import idempotent, and a drink deleted in the source app disappears here on + /// the next sync instead of being stranded. Retention pruning still happens on load. + public func replaceImported(_ imported: [CaffeineIntake]) { + let manual = intakes.filter { !$0.isImported } + let next = (manual + imported).sorted { $0.at > $1.at } + // Skip the write when nothing actually changed — `intakes` has a didSet that persists, and this + // runs on every sync, so an unconditional assignment would rewrite the JSON (and republish to + // every observing view) even on the overwhelmingly common no-op sync. + if next != intakes { intakes = next } + } + /// Remove a logged intake (the user mis-logged / wants to clear it). + /// + /// Imported intakes are deliberately NOT removable: the next sync re-reads the same window from + /// Apple Health and would bring it straight back, so honouring the tap would be a lie. The UI does + /// not offer the control; this guard means a caller that tries anyway gets a no-op rather than an + /// entry that silently reappears. public func remove(_ id: UUID) { + guard let hit = intakes.first(where: { $0.id == id }), !hit.isImported else { return } intakes = intakes.filter { $0.id != id } } - /// Clear every logged intake. + /// Clear every HAND-LOGGED intake. + /// + /// Imported ones survive for the same reason `remove` refuses them (#949) — clearing them here would + /// only last until the next sync re-read the window, so the list would silently repopulate and the + /// button would look broken. public func clearAll() { - intakes = [] + intakes = intakes.filter { $0.isImported } } /// The current "still active" estimate, computed from the logged intakes at `now()`. diff --git a/Strand/Screens/CaffeineLogCard.swift b/Strand/Screens/CaffeineLogCard.swift index 457305e648..4cdd835796 100644 --- a/Strand/Screens/CaffeineLogCard.swift +++ b/Strand/Screens/CaffeineLogCard.swift @@ -10,8 +10,9 @@ import StrandDesign /// unknown (we never invent mg), the active hint covers the dose-unknown case in words, and the copy /// states it's an estimate from what was logged. struct CaffeineLogCard: View { - /// Single-user state owned here (UserDefaults-backed), so hosting needs no app-level injection. - @StateObject private var store = CaffeineLogStore() + /// The shared UserDefaults-backed store (#949). Shared rather than owned here so the Apple Health + /// import and this card write through the same instance — see `CaffeineLogStore.shared`. + @ObservedObject private var store = CaffeineLogStore.shared /// Drives a live recompute of the estimate while the card is on screen (the decay is time-based). @State private var tick = Date() @@ -278,15 +279,24 @@ struct CaffeineLogCard: View { .font(StrandFont.body) .foregroundStyle(StrandPalette.textPrimary) Spacer() - Button { - store.remove(intake.id) - } label: { - Image(systemName: "minus.circle.fill") - .font(StrandFont.body) - .foregroundStyle(StrandPalette.statusCritical) + // No remove control on an imported intake (#949): the next sync re-reads the same window + // from Apple Health and would bring it straight back, so offering the button would be + // offering something NOOP cannot honour. Remove it where it was logged. + if intake.isImported { + Text("Apple Health") + .font(StrandFont.caption) + .foregroundStyle(StrandPalette.textTertiary) + } else { + Button { + store.remove(intake.id) + } label: { + Image(systemName: "minus.circle.fill") + .font(StrandFont.body) + .foregroundStyle(StrandPalette.statusCritical) + } + .buttonStyle(.plain) + .accessibilityLabel("Remove caffeine intake at \(Self.timeFormatter.string(from: intake.at))") } - .buttonStyle(.plain) - .accessibilityLabel("Remove caffeine intake at \(Self.timeFormatter.string(from: intake.at))") } } } diff --git a/StrandTests/CaffeineImportTests.swift b/StrandTests/CaffeineImportTests.swift new file mode 100644 index 0000000000..aedc4d3c82 --- /dev/null +++ b/StrandTests/CaffeineImportTests.swift @@ -0,0 +1,179 @@ +import XCTest +@testable import Strand + +/// #949 — caffeine imported from Apple Health sits in the same log as hand-entered intakes, and these +/// pin the rules that keep the two from corrupting each other. +/// +/// The shape of the problem is the same one the imported hydration row solves, but harder: hydration is +/// a single day total that can simply be replaced, whereas caffeine is a LIST of individual events. So +/// imported intakes carry the HealthKit sample UUID in `externalId`, and a sync replaces the imported +/// subset wholesale — that is what makes a re-import idempotent instead of logging a second copy of +/// every coffee, and what lets a drink deleted in the source app disappear here too. +/// +/// Headless: a private UserDefaults suite per test, no HealthKit, no UI. +final class CaffeineImportTests: XCTestCase { + + @MainActor + private func store(_ now: Date = Date()) -> CaffeineLogStore { + let suite = UserDefaults(suiteName: "caffeine.import.test.\(UUID().uuidString)")! + suite.removePersistentDomain(forName: suite.description) + return CaffeineLogStore(defaults: suite, now: { now }) + } + + private func imported(_ at: Date, mg: Double, id: String) -> CaffeineIntake { + CaffeineIntake(at: at, mg: mg, externalId: id) + } + + // MARK: - the model + + @MainActor + func testAHandLoggedIntakeIsNotImported() { + XCTAssertFalse(CaffeineIntake(at: Date(), mg: 80).isImported) + } + + @MainActor + func testAnIntakeWithAnExternalIdIsImported() { + XCTAssertTrue(imported(Date(), mg: 80, id: "abc").isImported) + } + + /// Stored JSON written before this change has no `externalId` field at all. It must still decode, and + /// every existing intake must read back as HAND-LOGGED — not as an import that the next sync would + /// then feel free to delete. + @MainActor + func testOldJsonWithoutExternalIdDecodesAsHandLogged() throws { + let json = #"[{"id":"\#(UUID().uuidString)","at":760000000,"mg":95}]"# + let decoded = try JSONDecoder().decode([CaffeineIntake].self, from: Data(json.utf8)) + XCTAssertEqual(decoded.count, 1) + XCTAssertNil(decoded[0].externalId) + XCTAssertFalse(decoded[0].isImported) + } + + // MARK: - replaceImported + + @MainActor + func testImportedIntakesAreAdded() { + let now = Date() + let s = store(now) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + XCTAssertEqual(s.intakes.count, 1) + XCTAssertEqual(s.intakes[0].mg, 95) + } + + /// The core idempotency claim: syncing the same Health window twice leaves ONE intake, not two. + @MainActor + func testReimportingTheSameSampleDoesNotDuplicateIt() { + let now = Date() + let s = store(now) + let sample = imported(now.addingTimeInterval(-3600), mg: 95, id: "a") + s.replaceImported([sample]) + s.replaceImported([sample]) + XCTAssertEqual(s.intakes.count, 1) + } + + /// Hand-logged intakes are never touched by a sync. + @MainActor + func testImportingLeavesHandLoggedIntakesAlone() { + let now = Date() + let s = store(now) + s.log(at: now.addingTimeInterval(-1800), mg: 60) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + XCTAssertEqual(s.intakes.count, 2) + XCTAssertEqual(s.intakes.filter { !$0.isImported }.count, 1) + XCTAssertEqual(s.intakes.filter { $0.isImported }.count, 1) + } + + /// Deleting the coffee in the app that logged it removes it here on the next sync, rather than + /// leaving it stranded in NOOP forever. + @MainActor + func testAnIntakeThatVanishesFromHealthIsDropped() { + let now = Date() + let s = store(now) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a"), + imported(now.addingTimeInterval(-7200), mg: 80, id: "b")]) + XCTAssertEqual(s.intakes.count, 2) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + XCTAssertEqual(s.intakes.count, 1) + XCTAssertEqual(s.intakes[0].externalId, "a") + } + + /// An empty Health window clears the imported set but must not touch what the user typed in. + @MainActor + func testAnEmptyImportClearsOnlyTheImportedOnes() { + let now = Date() + let s = store(now) + s.log(at: now.addingTimeInterval(-1800), mg: 60) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + s.replaceImported([]) + XCTAssertEqual(s.intakes.count, 1) + XCTAssertFalse(s.intakes[0].isImported) + } + + @MainActor + func testIntakesStayNewestFirstAfterAnImport() { + let now = Date() + let s = store(now) + s.log(at: now.addingTimeInterval(-7200), mg: 60) + s.replaceImported([imported(now.addingTimeInterval(-1800), mg: 95, id: "a")]) + XCTAssertEqual(s.intakes.map(\.at), s.intakes.map(\.at).sorted(by: >)) + } + + // MARK: - imported intakes are not the user's to edit + + /// Honouring this would be a lie: the next sync re-reads the same window and brings it back. + @MainActor + func testRemoveRefusesAnImportedIntake() { + let now = Date() + let s = store(now) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + let id = s.intakes[0].id + s.remove(id) + XCTAssertEqual(s.intakes.count, 1, "an imported intake must survive a remove") + } + + @MainActor + func testRemoveStillWorksOnAHandLoggedIntake() { + let now = Date() + let s = store(now) + s.log(at: now.addingTimeInterval(-1800), mg: 60) + s.remove(s.intakes[0].id) + XCTAssertTrue(s.intakes.isEmpty) + } + + /// "Clear" clears what the user owns. Wiping the imported ones would only last until the next sync. + @MainActor + func testClearAllKeepsImportedAndClearsHandLogged() { + let now = Date() + let s = store(now) + s.log(at: now.addingTimeInterval(-1800), mg: 60) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "a")]) + s.clearAll() + XCTAssertEqual(s.intakes.count, 1) + XCTAssertTrue(s.intakes[0].isImported) + } + + // MARK: - the estimate consumes imported intakes like any other + + @MainActor + func testImportedIntakesFeedTheActiveEstimate() { + let now = Date() + let s = store(now) + s.replaceImported([imported(now.addingTimeInterval(-3600), mg: 100, id: "a")]) + let est = s.estimate() + XCTAssertTrue(est.hasActive) + XCTAssertNotNil(est.totalRemainingMg, "an imported sample always carries a dose") + } + + /// Persistence round-trip: a fresh store over the same defaults sees the imported intake, and still + /// knows it was imported. + @MainActor + func testImportedIntakesSurviveAReload() throws { + let now = Date() + let suite = UserDefaults(suiteName: "caffeine.import.test.\(UUID().uuidString)")! + let a = CaffeineLogStore(defaults: suite, now: { now }) + a.replaceImported([imported(now.addingTimeInterval(-3600), mg: 95, id: "sample-1")]) + let b = CaffeineLogStore(defaults: suite, now: { now }) + XCTAssertEqual(b.intakes.count, 1) + XCTAssertEqual(b.intakes[0].externalId, "sample-1") + XCTAssertTrue(b.intakes[0].isImported) + } +} diff --git a/StrandiOS/Health/HealthKitBridge.swift b/StrandiOS/Health/HealthKitBridge.swift index 0a803d3fd2..5237acda13 100644 --- a/StrandiOS/Health/HealthKitBridge.swift +++ b/StrandiOS/Health/HealthKitBridge.swift @@ -104,7 +104,13 @@ final class HealthKitBridge: ObservableObject { // Water — READ-ONLY (#949), so drinks logged in a dedicated hydration app (or by a smart bottle) // show up without being typed in twice. Lands in the hydration source rather than apple-health, // because the hydration screen is what consumes it. Never written back. - .dietaryWater + .dietaryWater, + // Caffeine — READ-ONLY (#949). Feeds the caffeine window's decay estimate, which already stores + // time + optional mg per intake, so a Health sample maps onto it directly. Apple exposes this as + // its own narrow type; Health Connect has no caffeine-only scope (caffeine is a field on + // NutritionRecord, behind READ_NUTRITION — the whole food log), which is why Android is not + // matched here. Never written back. + .dietaryCaffeine ] private static let quantityWriteIds: [HKQuantityTypeIdentifier] = [ .restingHeartRate, .heartRateVariabilitySDNN, .oxygenSaturation, .respiratoryRate @@ -517,6 +523,18 @@ final class HealthKitBridge: ObservableObject { for (day, a) in byDay { if let ml = a.waterMl { waterByDay[day] = ml } } await repo.setImportedHydration(waterByDay) } + // Imported caffeine (#949) — only the last `CaffeineLogStore.retentionHours`, because the + // store prunes past that on load and the decay estimate is long dead by then. Reading the + // full 30-day sync window would hand over hundreds of samples for them all to be dropped. + // + // The caffeine card is manual-first and shows nothing until something is logged, so unlike + // hydration there is no separate opt-in toggle to gate on: an empty Health cupboard still + // leaves the card exactly as quiet as it is today. + if let caffeineStart = cal.date(byAdding: .hour, + value: -Int(CaffeineLogStore.retentionHours), to: end) { + let imported = await collectCaffeine(start: caffeineStart, end: end) + CaffeineLogStore.shared.replaceImported(imported) + } try await writeBack(whoopStore: store) lastSync = Date() lastError = nil @@ -981,6 +999,38 @@ final class HealthKitBridge: ObservableObject { /// the macOS export importer and the Android Health Connect importer, which already ingest workouts, /// closing the iOS gap. The upsert is idempotent on (deviceId, startTs), so re-running a sync window /// refreshes rather than duplicates. + /// Individual caffeine samples in the window, newest first (#949). + /// + /// A SAMPLE query, not the day-bucketed `collect`: the caffeine card estimates what is still active + /// from a half-life decay, so it needs each intake's own timestamp. A day total would collapse a 7am + /// coffee and a 9pm one into a single number that answers no question the card asks. + /// + /// Each sample keeps its HealthKit UUID as `externalId` so a re-import can tell an intake it already + /// has from a new one. `notNoopAuthored` keeps NOOP's own samples out — we do not write caffeine + /// today, but the predicate costs nothing and closes the loop if that ever changes. + private func collectCaffeine(start: Date, end: Date) async -> [CaffeineIntake] { + guard let type = HKQuantityType.quantityType(forIdentifier: .dietaryCaffeine) else { return [] } + let predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [ + HKQuery.predicateForSamples(withStart: start, end: end, options: .strictStartDate), + Self.notNoopAuthored, + ]) + return await withCheckedContinuation { (cont: CheckedContinuation<[CaffeineIntake], Never>) in + let sort = NSSortDescriptor(key: HKSampleSortIdentifierStartDate, ascending: false) + let q = HKSampleQuery(sampleType: type, predicate: predicate, + limit: HKObjectQueryNoLimit, sortDescriptors: [sort]) { _, samples, _ in + let out: [CaffeineIntake] = (samples as? [HKQuantitySample] ?? []).compactMap { s in + let mg = s.quantity.doubleValue(for: .gramUnit(with: .milli)) + // A zero or non-finite sample carries no dose worth showing; skip it rather than log + // a 0 mg intake, which would pad the "intakes still active" count with nothing. + guard mg.isFinite, mg > 0 else { return nil } + return CaffeineIntake(at: s.startDate, mg: mg, externalId: s.uuid.uuidString) + } + cont.resume(returning: out) + } + store.execute(q) + } + } + private func collectWorkouts(start: Date, end: Date) async -> [WorkoutRow] { let predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [ HKQuery.predicateForSamples(withStart: start, end: end, options: .strictStartDate), From 7e0ba0f0383fd614878f0238d38fc129866ae61d Mon Sep 17 00:00:00 2001 From: ryanbr Date: Thu, 30 Jul 2026 12:30:02 -0700 Subject: [PATCH 2/2] Caffeine: stable ids for imported intakes, and don't wipe them on a read error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- StrandTests/CaffeineImportTests.swift | 54 ++++++++++++++++++++++++-- StrandiOS/Health/HealthKitBridge.swift | 27 +++++++++---- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/StrandTests/CaffeineImportTests.swift b/StrandTests/CaffeineImportTests.swift index aedc4d3c82..ce68cdda73 100644 --- a/StrandTests/CaffeineImportTests.swift +++ b/StrandTests/CaffeineImportTests.swift @@ -24,6 +24,16 @@ final class CaffeineImportTests: XCTestCase { CaffeineIntake(at: at, mg: mg, externalId: id) } + /// A fixed stand-in for a HealthKit sample uuid. + private let sampleUUID = UUID(uuidString: "11111111-2222-3333-4444-555555555555")! + + /// Builds an intake exactly the way `HealthKitBridge.collectCaffeine` does — the sample's own uuid + /// as BOTH the identity and the external marker. Kept in one place so a drift between this and the + /// bridge is a one-line fix rather than a silently passing test. + private func fromSample(uuid: UUID, at: Date, mg: Double) -> CaffeineIntake { + CaffeineIntake(id: uuid, at: at, mg: mg, externalId: uuid.uuidString) + } + // MARK: - the model @MainActor @@ -60,16 +70,54 @@ final class CaffeineImportTests: XCTestCase { } /// The core idempotency claim: syncing the same Health window twice leaves ONE intake, not two. + /// + /// Built from a fresh `CaffeineIntake` each time rather than reusing one value, because that is what + /// the bridge actually does — it rebuilds every intake from the HealthKit samples on each sync. An + /// earlier version of this test reused a single value and would have passed while the real path was + /// broken. @MainActor func testReimportingTheSameSampleDoesNotDuplicateIt() { let now = Date() let s = store(now) - let sample = imported(now.addingTimeInterval(-3600), mg: 95, id: "a") - s.replaceImported([sample]) - s.replaceImported([sample]) + let at = now.addingTimeInterval(-3600) + s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)]) + s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)]) XCTAssertEqual(s.intakes.count, 1) } + /// The identity of an imported intake must come from the SAMPLE, not be freshly minted per sync. + /// + /// This is the bug the test above could not see: `CaffeineIntake.id` defaults to `UUID()`, so + /// building one per sync gave the same coffee a new identity every time — `replaceImported`'s + /// "nothing changed" guard never matched (a pointless rewrite and republish on every sync) and + /// `ForEach` rebuilt the whole logged list. Two intakes built from one sample must be EQUAL. + @MainActor + func testTwoIntakesBuiltFromTheSameSampleAreEqual() { + let at = Date().addingTimeInterval(-3600) + XCTAssertEqual(fromSample(uuid: sampleUUID, at: at, mg: 95), + fromSample(uuid: sampleUUID, at: at, mg: 95)) + } + + /// ...and the id is the sample's, so SwiftUI sees a stable row across syncs. + @MainActor + func testImportedIntakeIdIsTheSampleId() { + let intake = fromSample(uuid: sampleUUID, at: Date(), mg: 95) + XCTAssertEqual(intake.id, sampleUUID) + XCTAssertEqual(intake.externalId, sampleUUID.uuidString) + } + + /// Re-importing an unchanged window must leave the stored array untouched, identities included. + @MainActor + func testReimportingAnUnchangedWindowKeepsTheSameIds() { + let now = Date() + let s = store(now) + let at = now.addingTimeInterval(-3600) + s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)]) + let before = s.intakes.map(\.id) + s.replaceImported([fromSample(uuid: sampleUUID, at: at, mg: 95)]) + XCTAssertEqual(before, s.intakes.map(\.id)) + } + /// Hand-logged intakes are never touched by a sync. @MainActor func testImportingLeavesHandLoggedIntakesAlone() { diff --git a/StrandiOS/Health/HealthKitBridge.swift b/StrandiOS/Health/HealthKitBridge.swift index 5237acda13..2294ad6f9e 100644 --- a/StrandiOS/Health/HealthKitBridge.swift +++ b/StrandiOS/Health/HealthKitBridge.swift @@ -532,8 +532,10 @@ final class HealthKitBridge: ObservableObject { // leaves the card exactly as quiet as it is today. if let caffeineStart = cal.date(byAdding: .hour, value: -Int(CaffeineLogStore.retentionHours), to: end) { - let imported = await collectCaffeine(start: caffeineStart, end: end) - CaffeineLogStore.shared.replaceImported(imported) + // nil means the READ failed; only an actual empty result is allowed to clear the set. + if let imported = await collectCaffeine(start: caffeineStart, end: end) { + CaffeineLogStore.shared.replaceImported(imported) + } } try await writeBack(whoopStore: store) lastSync = Date() @@ -1008,22 +1010,33 @@ final class HealthKitBridge: ObservableObject { /// Each sample keeps its HealthKit UUID as `externalId` so a re-import can tell an intake it already /// has from a new one. `notNoopAuthored` keeps NOOP's own samples out — we do not write caffeine /// today, but the predicate costs nothing and closes the loop if that ever changes. - private func collectCaffeine(start: Date, end: Date) async -> [CaffeineIntake] { - guard let type = HKQuantityType.quantityType(forIdentifier: .dietaryCaffeine) else { return [] } + /// + /// Returns nil when the read FAILED, as distinct from an empty array meaning "no caffeine logged". + /// The caller replaces the imported set wholesale, so collapsing those two cases would let a failed + /// query silently delete every imported intake. + private func collectCaffeine(start: Date, end: Date) async -> [CaffeineIntake]? { + guard let type = HKQuantityType.quantityType(forIdentifier: .dietaryCaffeine) else { return nil } let predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [ HKQuery.predicateForSamples(withStart: start, end: end, options: .strictStartDate), Self.notNoopAuthored, ]) - return await withCheckedContinuation { (cont: CheckedContinuation<[CaffeineIntake], Never>) in + return await withCheckedContinuation { (cont: CheckedContinuation<[CaffeineIntake]?, Never>) in let sort = NSSortDescriptor(key: HKSampleSortIdentifierStartDate, ascending: false) let q = HKSampleQuery(sampleType: type, predicate: predicate, - limit: HKObjectQueryNoLimit, sortDescriptors: [sort]) { _, samples, _ in + limit: HKObjectQueryNoLimit, sortDescriptors: [sort]) { _, samples, error in + guard error == nil, let samples else { cont.resume(returning: nil); return } let out: [CaffeineIntake] = (samples as? [HKQuantitySample] ?? []).compactMap { s in let mg = s.quantity.doubleValue(for: .gramUnit(with: .milli)) // A zero or non-finite sample carries no dose worth showing; skip it rather than log // a 0 mg intake, which would pad the "intakes still active" count with nothing. guard mg.isFinite, mg > 0 else { return nil } - return CaffeineIntake(at: s.startDate, mg: mg, externalId: s.uuid.uuidString) + // The sample's OWN uuid as the intake id, not a fresh one. `CaffeineIntake` defaults + // `id` to `UUID()`, so minting one here would give the same coffee a different + // identity on every sync: `replaceImported`'s no-op guard would never match (a + // pointless JSON rewrite + republish each time), and `ForEach` would see the whole + // logged list as new rows and rebuild it. HealthKit sample uuids are stable. + return CaffeineIntake(id: s.uuid, at: s.startDate, mg: mg, + externalId: s.uuid.uuidString) } cont.resume(returning: out) }