From 3bfd7d6f44a6feba0804ec68af8d6e5d1d75eb43 Mon Sep 17 00:00:00 2001 From: ak710 Date: Sat, 1 Aug 2026 23:39:42 -0400 Subject: [PATCH 1/5] Keep date keys Gregorian regardless of the device calendar A DateFormatter takes its calendar from the user's locale, so on a device set to the Buddhist or Japanese calendar (Settings -> General -> Language & Region -> Calendar) "yyyy-MM-dd" renders 1 Aug 2026 as 2569-08-01 or 8-08-01. That is correct for display and wrong for the strings this touches, which are identifiers rather than text: coach summary scope keys, the notification dedupe key, export and share-card filenames, and the date arguments the coach emits and parses back. A key that follows the calendar setting stops matching the keys already in the store, stops sorting chronologically against them, and stops being a date the model can read back. Adds DateFormatter.stableKey, which pins en_US_POSIX + Gregorian -- the same combination BatteryAlertMonitor has always used for its own dedupe key -- and routes the affected call sites through it. Co-Authored-By: Claude Opus 5 --- .../Coach/Context/CoachContextBuilder.swift | 5 +- .../CoachNotificationModels.swift | 9 +- PulseLoop/Coach/Tools/AnalysisEngine.swift | 2 +- PulseLoop/Coach/Tools/CoachDataAccess.swift | 20 +--- .../Persistence/DataArchiveService.swift | 3 +- PulseLoop/Services/DateFormatting.swift | 58 ++++++++++ PulseLoop/Sharing/ShareCardRenderer.swift | 3 +- PulseLoopTests/DateFormattingTests.swift | 106 ++++++++++++++++++ 8 files changed, 176 insertions(+), 30 deletions(-) create mode 100644 PulseLoop/Services/DateFormatting.swift create mode 100644 PulseLoopTests/DateFormattingTests.swift diff --git a/PulseLoop/Coach/Context/CoachContextBuilder.swift b/PulseLoop/Coach/Context/CoachContextBuilder.swift index 9b7a42af..1cce1177 100644 --- a/PulseLoop/Coach/Context/CoachContextBuilder.swift +++ b/PulseLoop/Coach/Context/CoachContextBuilder.swift @@ -224,10 +224,7 @@ enum CoachContextBuilder { private static func iso(_ date: Date) -> String { isoFormatter.string(from: date) } private static func localDate(_ date: Date) -> String { - let f = DateFormatter() - f.dateFormat = "yyyy-MM-dd" - f.timeZone = .current - return f.string(from: date) + DateFormatter.stableKey("yyyy-MM-dd").string(from: date) } } diff --git a/PulseLoop/Coach/Notifications/CoachNotificationModels.swift b/PulseLoop/Coach/Notifications/CoachNotificationModels.swift index 064b0d3b..2f079124 100644 --- a/PulseLoop/Coach/Notifications/CoachNotificationModels.swift +++ b/PulseLoop/Coach/Notifications/CoachNotificationModels.swift @@ -67,11 +67,10 @@ final class CoachNotificationRecord { var slot: CoachNotificationSlot { CoachNotificationSlot(rawValue: slotRaw) ?? .morning } + /// The dedupe key for "has this slot already fired today". `calendar` decides *which* local day + /// a timestamp falls in; the rendering itself is pinned Gregorian so the key stays comparable to + /// the ones already stored (see `DateFormatter.stableKey`). static func dateKey(for date: Date, calendar: Calendar = .current) -> String { - let f = DateFormatter() - f.dateFormat = "yyyy-MM-dd" - f.calendar = calendar - f.timeZone = calendar.timeZone - return f.string(from: date) + DateFormatter.stableKey("yyyy-MM-dd", timeZone: calendar.timeZone).string(from: date) } } diff --git a/PulseLoop/Coach/Tools/AnalysisEngine.swift b/PulseLoop/Coach/Tools/AnalysisEngine.swift index 4462755c..4f2efcca 100644 --- a/PulseLoop/Coach/Tools/AnalysisEngine.swift +++ b/PulseLoop/Coach/Tools/AnalysisEngine.swift @@ -131,7 +131,7 @@ enum AnalysisEngine { let mean = values.reduce(0, +) / n let sd = sqrt(values.reduce(0) { $0 + pow($1 - mean, 2) } / n) guard sd > 0 else { return [] } - let f = DateFormatter(); f.dateFormat = "yyyy-MM-dd"; f.timeZone = .current + let f = DateFormatter.stableKey("yyyy-MM-dd") return series.compactMap { item in let z = (item.value - mean) / sd guard abs(z) >= threshold else { return nil } diff --git a/PulseLoop/Coach/Tools/CoachDataAccess.swift b/PulseLoop/Coach/Tools/CoachDataAccess.swift index c424284b..4ef1ba55 100644 --- a/PulseLoop/Coach/Tools/CoachDataAccess.swift +++ b/PulseLoop/Coach/Tools/CoachDataAccess.swift @@ -17,27 +17,18 @@ enum CoachDataAccess { static func parseLocalDate(_ value: String) -> Date? { let trimmed = String(value.prefix(10)) - let f = DateFormatter() - f.dateFormat = "yyyy-MM-dd" - f.timeZone = .current - if let d = f.date(from: trimmed) { return d } + if let d = DateFormatter.stableKey("yyyy-MM-dd").date(from: trimmed) { return d } // Fall back to ISO datetime. let iso = ISO8601DateFormatter() return iso.date(from: value) } static func localDateString(_ date: Date) -> String { - let f = DateFormatter() - f.dateFormat = "yyyy-MM-dd" - f.timeZone = .current - return f.string(from: date) + DateFormatter.stableKey("yyyy-MM-dd").string(from: date) } static func localTimeString(_ date: Date) -> String { - let f = DateFormatter() - f.dateFormat = "HH:mm" - f.timeZone = .current - return f.string(from: date) + DateFormatter.stableKey("HH:mm").string(from: date) } static func isoString(_ date: Date) -> String { @@ -177,9 +168,6 @@ enum CoachDataAccess { } private static func hourLabel(_ date: Date) -> String { - let f = DateFormatter() - f.dateFormat = "yyyy-MM-dd HH:00" - f.timeZone = .current - return f.string(from: date) + DateFormatter.stableKey("yyyy-MM-dd HH:00").string(from: date) } } diff --git a/PulseLoop/Persistence/DataArchiveService.swift b/PulseLoop/Persistence/DataArchiveService.swift index e53ce010..cc08080a 100644 --- a/PulseLoop/Persistence/DataArchiveService.swift +++ b/PulseLoop/Persistence/DataArchiveService.swift @@ -81,8 +81,7 @@ enum DataArchiveService { /// Exports to a shareable temp file (`pulseloop-export-.json`) for the share sheet. static func exportFile(context: ModelContext) async throws -> URL { let data = try await exportArchive(context: context) - let formatter = DateFormatter() - formatter.dateFormat = "yyyy-MM-dd-HHmm" + let formatter = DateFormatter.stableKey("yyyy-MM-dd-HHmm") let url = FileManager.default.temporaryDirectory .appendingPathComponent("pulseloop-export-\(formatter.string(from: Date())).json") try data.write(to: url, options: .atomic) diff --git a/PulseLoop/Services/DateFormatting.swift b/PulseLoop/Services/DateFormatting.swift new file mode 100644 index 00000000..c436b6a0 --- /dev/null +++ b/PulseLoop/Services/DateFormatting.swift @@ -0,0 +1,58 @@ +import Foundation + +// Two jobs that look identical at the call site and fail in opposite directions when confused. +// Both bugs below are invisible on a default US device, which is why they survived this long. + +extension DateFormatter { + /// A formatter for strings that are **identifiers, not display text**: coach summary scope keys, + /// notification dedupe keys, export filenames, and the date arguments the coach emits and parses + /// back. + /// + /// A `DateFormatter` takes its calendar from the user's locale. On a device set to the Buddhist + /// or Japanese calendar (Settings → General → Language & Region → Calendar), `"yyyy-MM-dd"` + /// renders 1 Aug 2026 as `2569-08-01` / `8-08-01`. For display that is correct and wanted; for a + /// key it is a bug — the string stops matching keys written before the setting changed, stops + /// sorting chronologically against them, and stops being a date the model can parse back. + /// + /// Pinning `en_US_POSIX` + Gregorian is the fix, and the combination `BatteryAlertMonitor` has + /// always used for its own alert-dedupe key. + /// + /// `timeZone` defaults to the device's, matching the "local day" these keys have always meant. + static func stableKey(_ format: String, timeZone: TimeZone = .current, + calendar: Calendar = Calendar(identifier: .gregorian)) -> DateFormatter { + let formatter = DateFormatter() + formatter.locale = Locale(identifier: "en_US_POSIX") + formatter.calendar = calendar + formatter.timeZone = timeZone + formatter.dateFormat = format + return formatter + } + + /// A formatter for text the **user reads**, built from a locale template rather than a literal + /// pattern. + /// + /// `"jmm"` resolves to `9:30 PM` or `21:30` according to the device's 24-Hour Time setting, + /// where a hard-coded `"h:mm a"` forces 12-hour on everyone — including the large share of the + /// world that has never used it. Templates also reorder fields per locale, so `"MMMd"` gives + /// `Aug 1` or `1 Aug` as appropriate. + /// + /// Field *letters* still matter (`j` hour, `mm` minute, `MMM` abbreviated month); only their + /// order and the 12/24-hour choice are handed to the locale. + static func localizedTemplate(_ template: String, locale: Locale = .current) -> DateFormatter { + let formatter = DateFormatter() + formatter.locale = locale + // Must follow the locale assignment: the template is resolved against it. + formatter.setLocalizedDateFormatFromTemplate(template) + return formatter + } + + /// Whether `locale` renders times on a 12-hour clock, and so has an AM/PM marker to place. + /// + /// Read from the locale's own resolution of the `j` ("locale-preferred hour") template — the + /// same thing Settings → General → Date & Time → 24-Hour Time flips. Callers need this only when + /// the *layout* depends on the marker existing; for plain formatting, `localizedTemplate("jmm")` + /// already does the right thing on both. + static func usesTwelveHourClock(locale: Locale = .current) -> Bool { + (dateFormat(fromTemplate: "j", options: 0, locale: locale) ?? "").contains("a") + } +} diff --git a/PulseLoop/Sharing/ShareCardRenderer.swift b/PulseLoop/Sharing/ShareCardRenderer.swift index f38cf9f6..40d03411 100644 --- a/PulseLoop/Sharing/ShareCardRenderer.swift +++ b/PulseLoop/Sharing/ShareCardRenderer.swift @@ -33,8 +33,7 @@ enum ShareCardRenderer { let slug = activityLabel.lowercased() .replacingOccurrences(of: "[^a-z0-9]+", with: "-", options: .regularExpression) .trimmingCharacters(in: CharacterSet(charactersIn: "-")) - let formatter = DateFormatter() - formatter.dateFormat = "yyyy-MM-dd" + let formatter = DateFormatter.stableKey("yyyy-MM-dd") return "pulseloop-\(slug.isEmpty ? "workout" : slug)-\(formatter.string(from: date)).png" } diff --git a/PulseLoopTests/DateFormattingTests.swift b/PulseLoopTests/DateFormattingTests.swift new file mode 100644 index 00000000..5fab63ff --- /dev/null +++ b/PulseLoopTests/DateFormattingTests.swift @@ -0,0 +1,106 @@ +import XCTest +@testable import PulseLoop + +/// Covers the split between date strings that are **keys** (`DateFormatter.stableKey`) and date +/// strings the **user reads** (`DateFormatter.localizedTemplate`). +/// +/// Both directions are invisible on a default US device, so they need pinning rather than eyeballing: +/// a key that follows the device calendar silently stops matching stored keys, and a display string +/// that ignores the device clock silently shows 12-hour time to a 24-hour user. +final class DateFormattingTests: XCTestCase { + private let utc = TimeZone(identifier: "UTC")! + + private func date(_ year: Int, _ month: Int, _ day: Int, hour: Int = 12, minute: Int = 0) -> Date { + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = utc + return calendar.date(from: DateComponents(year: year, month: month, day: day, hour: hour, minute: minute))! + } + + // MARK: - Keys stay Gregorian + + /// The exact regression: a formatter that inherits a non-Gregorian calendar renders 2026 as the + /// Buddhist year 2569, so the key stops matching every key written before the setting changed. + /// `stableKey` must be immune to that. + func testStableKeyIgnoresANonGregorianCalendar() { + let august1st2026 = date(2026, 8, 1) + + // What the old code produced on a Thai device, reproduced explicitly so the test doesn't + // depend on ICU's default calendar for any particular locale. + let deviceCalendarFormatter = DateFormatter() + deviceCalendarFormatter.locale = Locale(identifier: "th_TH") + deviceCalendarFormatter.calendar = Calendar(identifier: .buddhist) + deviceCalendarFormatter.timeZone = utc + deviceCalendarFormatter.dateFormat = "yyyy-MM-dd" + XCTAssertEqual(deviceCalendarFormatter.string(from: august1st2026), "2569-08-01", + "Precondition: a Buddhist-calendar formatter renders a different year") + + XCTAssertEqual(DateFormatter.stableKey("yyyy-MM-dd", timeZone: utc).string(from: august1st2026), + "2026-08-01") + } + + /// Keys must also sort chronologically as plain strings, which a two-era mix would break. + func testStableKeysSortChronologicallyAsStrings() { + let formatter = DateFormatter.stableKey("yyyy-MM-dd", timeZone: utc) + let keys = [date(2026, 8, 1), date(2025, 12, 31), date(2026, 1, 1)].map(formatter.string(from:)) + XCTAssertEqual(keys.sorted(), ["2025-12-31", "2026-01-01", "2026-08-01"]) + } + + @MainActor + func testCoachDateStringsRoundTripThroughTheirOwnParser() { + let noon = date(2026, 8, 1) + let key = CoachDataAccess.localDateString(noon) + XCTAssertEqual(key.count, 10, "Coach day keys are YYYY-MM-DD: \(key)") + + let parsed = CoachDataAccess.parseLocalDate(key) + XCTAssertNotNil(parsed) + // Parsing a day key yields that day's local midnight, which re-renders to the same key. + XCTAssertEqual(CoachDataAccess.localDateString(parsed!), key) + } + + func testNotificationDedupeKeyIsGregorian() { + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = utc + XCTAssertEqual(CoachNotificationRecord.dateKey(for: date(2026, 8, 1), calendar: calendar), "2026-08-01") + } + + func testShareCardFilenameCarriesAGregorianDate() { + let filename = ShareCardRenderer.filename(activityLabel: "Trail Run", date: date(2026, 8, 1)) + XCTAssertTrue(filename.hasSuffix(".png"), filename) + XCTAssertTrue(filename.contains("2026-08-"), "Expected a Gregorian year in \(filename)") + } + + // MARK: - Display follows the device + + /// A hard-coded "h:mm a" showed 12-hour time to everyone. The template has to resolve per locale. + func testLocalizedTemplateFollowsTheLocaleClock() { + let evening = date(2026, 8, 1, hour: 21, minute: 30) + + let twelveHour = DateFormatter.localizedTemplate("jmm", locale: Locale(identifier: "en_US")) + twelveHour.timeZone = utc + let twelveHourText = twelveHour.string(from: evening) + XCTAssertTrue(twelveHourText.contains("9:30"), twelveHourText) + XCTAssertTrue(twelveHourText.uppercased().contains("PM"), twelveHourText) + + let twentyFourHour = DateFormatter.localizedTemplate("jmm", locale: Locale(identifier: "de_DE")) + twentyFourHour.timeZone = utc + let twentyFourHourText = twentyFourHour.string(from: evening) + XCTAssertTrue(twentyFourHourText.contains("21:30"), twentyFourHourText) + XCTAssertFalse(twentyFourHourText.uppercased().contains("PM"), twentyFourHourText) + } + + func testUsesTwelveHourClockTracksTheLocale() { + XCTAssertTrue(DateFormatter.usesTwelveHourClock(locale: Locale(identifier: "en_US"))) + XCTAssertFalse(DateFormatter.usesTwelveHourClock(locale: Locale(identifier: "de_DE"))) + } + + /// Sleep bed/wake times are the most visible clock text in the app. + func testSleepClockTimeIsNotHardCodedToTwelveHour() { + let formatted = SleepFormat.clockTime(date(2026, 8, 1, hour: 23, minute: 15)) + if DateFormatter.usesTwelveHourClock() { + XCTAssertTrue(formatted.uppercased().contains("M"), formatted) + } else { + XCTAssertFalse(formatted.uppercased().contains("AM"), formatted) + XCTAssertFalse(formatted.uppercased().contains("PM"), formatted) + } + } +} From ee6c5da0bcff1a576973d87bf0eb9e859d916d98 Mon Sep 17 00:00:00 2001 From: ak710 Date: Sat, 1 Aug 2026 23:39:57 -0400 Subject: [PATCH 2/5] Follow the device's 24-hour time setting Six display sites hard-coded "h:mm a", which forces 12-hour time on everyone -- including users who have turned on Settings -> General -> Date & Time -> 24-Hour Time, and the many locales where 24-hour is the norm. It showed up on the sleep stage chart axis, sleep bed/wake times, the workout summary header, meal detail, and the share card. Replaces the literal patterns with locale templates via DateFormatter.localizedTemplate: "jmm" resolves to 9:30 PM or 21:30 per device. The workout summary carries its AM/PM marker once, on the end of the range ("7:32 - 8:05 AM"), so it asks the locale whether there is a marker to place at all rather than assuming one. Month/day patterns alongside them move to templates too, which also gets their field order right per locale ("Aug 1" vs "1 Aug"). Co-Authored-By: Claude Opus 5 --- PulseLoop/DesignSystem/Charts.swift | 3 +-- PulseLoop/Services/SleepInsights.swift | 6 +----- PulseLoop/Sharing/ShareCardModel.swift | 6 +++--- PulseLoop/Views/Nutrition/MealDetailView.swift | 15 +++++++++------ PulseLoop/Views/RecordSummaryComponents.swift | 11 +++++++---- 5 files changed, 21 insertions(+), 20 deletions(-) diff --git a/PulseLoop/DesignSystem/Charts.swift b/PulseLoop/DesignSystem/Charts.swift index 8b657691..fe512855 100644 --- a/PulseLoop/DesignSystem/Charts.swift +++ b/PulseLoop/DesignSystem/Charts.swift @@ -455,8 +455,7 @@ struct SleepHypnogramView: View { private var ticks: [(offset: Int, label: String)] { let safe = totalMin > 0 ? totalMin : 1 let offsets = [0, safe / 3, safe * 2 / 3, safe] - let formatter = DateFormatter() - formatter.dateFormat = "h:mm a" + let formatter = DateFormatter.localizedTemplate("jmm") return offsets.map { offset in if let start = startTs { let date = start.addingTimeInterval(Double(offset) * 60) diff --git a/PulseLoop/Services/SleepInsights.swift b/PulseLoop/Services/SleepInsights.swift index d75a91f7..8a9cf182 100644 --- a/PulseLoop/Services/SleepInsights.swift +++ b/PulseLoop/Services/SleepInsights.swift @@ -113,11 +113,7 @@ enum SleepFormat { return "\(h)h \(String(format: "%02d", m))m" } - private static let clockTimeFormatter: DateFormatter = { - let f = DateFormatter() - f.dateFormat = "h:mm a" - return f - }() + private static let clockTimeFormatter = DateFormatter.localizedTemplate("jmm") static func clockTime(_ date: Date) -> String { clockTimeFormatter.string(from: date) diff --git a/PulseLoop/Sharing/ShareCardModel.swift b/PulseLoop/Sharing/ShareCardModel.swift index 710f4316..979baa19 100644 --- a/PulseLoop/Sharing/ShareCardModel.swift +++ b/PulseLoop/Sharing/ShareCardModel.swift @@ -182,9 +182,9 @@ struct ShareCardModel { // MARK: - Dates private static func dateHeadline(_ date: Date) -> String { - let dow = DateFormatter(); dow.dateFormat = "EEE" - let monthDay = DateFormatter(); monthDay.dateFormat = "MMM d" - let time = DateFormatter(); time.dateFormat = "h:mm a" + let dow = DateFormatter.localizedTemplate("EEE") + let monthDay = DateFormatter.localizedTemplate("MMMd") + let time = DateFormatter.localizedTemplate("jmm") return "\(dow.string(from: date).uppercased()) · \(monthDay.string(from: date).uppercased()) · \(time.string(from: date))" } diff --git a/PulseLoop/Views/Nutrition/MealDetailView.swift b/PulseLoop/Views/Nutrition/MealDetailView.swift index 8b4c81ab..0052f621 100644 --- a/PulseLoop/Views/Nutrition/MealDetailView.swift +++ b/PulseLoop/Views/Nutrition/MealDetailView.swift @@ -87,11 +87,14 @@ struct MealDetailView: View { // MARK: - Sections - private static let timeFormatter: DateFormatter = { - let f = DateFormatter() - f.dateFormat = "EEE, MMM d · h:mm a" - return f - }() + // Two formatters rather than one pattern: the " · " is ours, but the day and time either side of + // it belong to the locale (field order, and 12- vs 24-hour). + private static let dayFormatter = DateFormatter.localizedTemplate("EEEMMMd") + private static let timeFormatter = DateFormatter.localizedTemplate("jmm") + + private static func timestampLabel(_ date: Date) -> String { + "\(dayFormatter.string(from: date)) · \(timeFormatter.string(from: date))" + } private func titleBlock(_ entry: MealEntry) -> some View { VStack(alignment: .leading, spacing: 6) { @@ -105,7 +108,7 @@ struct MealDetailView: View { .background(PulseColors.calories.opacity(0.14), in: Capsule()) ProvenanceBadge(source: entry.source, userEdited: entry.userEdited) } - Text(Self.timeFormatter.string(from: entry.timestamp)) + Text(Self.timestampLabel(entry.timestamp)) .font(PulseFont.caption.weight(.regular)) .foregroundStyle(PulseColors.textMuted) } diff --git a/PulseLoop/Views/RecordSummaryComponents.swift b/PulseLoop/Views/RecordSummaryComponents.swift index 7ba0f121..97942519 100644 --- a/PulseLoop/Views/RecordSummaryComponents.swift +++ b/PulseLoop/Views/RecordSummaryComponents.swift @@ -165,17 +165,20 @@ struct WorkoutMetricsSections: View { .padding(.top, 8) } - /// e.g. "Today · 7:32 – 8:05 AM" or "May 28 · 6:10 – 6:48 PM". + /// e.g. "Today · 7:32 – 8:05 AM", or "Today · 07:32 – 08:05" where the device is on 24-hour time. + /// + /// The AM/PM marker is carried once, on the end of the range. A 24-hour locale has no marker to + /// carry, so both ends format the same way there. private var dateRange: String { - let time = DateFormatter(); time.dateFormat = "h:mm" - let timeAmPm = DateFormatter(); timeAmPm.dateFormat = "h:mm a" + let time = DateFormatter.localizedTemplate(DateFormatter.usesTwelveHourClock() ? "hmm" : "jmm") + let timeAmPm = DateFormatter.localizedTemplate("jmm") let day: String if Calendar.current.isDateInToday(session.startedAt) { day = "Today" } else if Calendar.current.isDateInYesterday(session.startedAt) { day = "Yesterday" } else { - let d = DateFormatter(); d.dateFormat = "MMM d" + let d = DateFormatter.localizedTemplate("MMMd") day = d.string(from: session.startedAt) } guard let ended = session.endedAt else { return day } From a057b06a34225c3c0ae20f9fc4bd10b2377f7ba9 Mon Sep 17 00:00:00 2001 From: ak710 Date: Sat, 1 Aug 2026 23:39:57 -0400 Subject: [PATCH 3/5] Fix two windowed reads that scanned or truncated wrongly SleepRepository.latestSession sorted the whole SleepSession table and took .first, with no fetchLimit -- every sibling in the file sets one (latestMeasurement, ReadinessRepository.latest, oldestMeasurementTimestamp). MetricsRepository.batterySamples sorted forward and then applied fetchLimit, so a window holding more than the cap returned the *oldest* rows and silently dropped the newest -- the opposite of what the drainage chart exists to show. It now fetches newest-first and reverses, keeping the documented oldest-first return order while letting the cap drop old rows instead of recent ones. Co-Authored-By: Claude Opus 5 --- PulseLoop/Services/Repositories.swift | 14 +++- .../RepositoryFetchLimitTests.swift | 76 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 PulseLoopTests/RepositoryFetchLimitTests.swift diff --git a/PulseLoop/Services/Repositories.swift b/PulseLoop/Services/Repositories.swift index 07d7b1bd..cb2f0d39 100644 --- a/PulseLoop/Services/Repositories.swift +++ b/PulseLoop/Services/Repositories.swift @@ -85,12 +85,16 @@ enum MetricsRepository { /// full-table scan. Feeds the Wearable screen's drainage chart. @MainActor static func batterySamples(start: Date, end: Date, limit: Int = 1000, context: ModelContext) -> [BatterySample] { + // Fetched newest-first and reversed, rather than fetched oldest-first: `limit` has to drop + // the *oldest* rows in an over-long window, not the newest. Sorting forward meant a busy + // window past the cap charted the start of the range and silently omitted the recent + // readings the drainage chart exists to show. The returned order is unchanged (oldest-first). var descriptor = FetchDescriptor( predicate: #Predicate { $0.timestamp >= start && $0.timestamp <= end }, - sortBy: [SortDescriptor(\.timestamp, order: .forward)] + sortBy: [SortDescriptor(\.timestamp, order: .reverse)] ) descriptor.fetchLimit = limit - return (try? context.fetch(descriptor)) ?? [] + return ((try? context.fetch(descriptor)) ?? []).reversed() } /// All measurements of one kind, newest-first (demo mode keeps full history, no time window). @@ -181,9 +185,11 @@ enum SleepRepository { } @MainActor + /// `fetchLimit: 1` — one row, not the whole table (matching `latestMeasurement` above). static func latestSession(context: ModelContext) -> SleepSession? { - let descriptor = FetchDescriptor(sortBy: [SortDescriptor(\.startAt, order: .reverse)]) - return (try? context.fetch(descriptor))?.first + var descriptor = FetchDescriptor(sortBy: [SortDescriptor(\.startAt, order: .reverse)]) + descriptor.fetchLimit = 1 + return try? context.fetch(descriptor).first } @MainActor diff --git a/PulseLoopTests/RepositoryFetchLimitTests.swift b/PulseLoopTests/RepositoryFetchLimitTests.swift new file mode 100644 index 00000000..1568884a --- /dev/null +++ b/PulseLoopTests/RepositoryFetchLimitTests.swift @@ -0,0 +1,76 @@ +import XCTest +import SwiftData +@testable import PulseLoop + +/// Covers what `fetchLimit` does to a windowed read: which rows it keeps, and which end of the +/// range it drops when the window holds more than the cap. +@MainActor +final class RepositoryFetchLimitTests: XCTestCase { + /// A capped window has to drop the *oldest* rows, not the newest. + /// + /// Sorting forward and then truncating meant a chart over a busy window rendered the beginning + /// of the range and silently omitted the recent readings — the opposite of what a drainage + /// chart is for. + func testBatterySamplesKeepTheNewestRowsWhenTruncated() throws { + let context = try TestSupport.makeContext() + let end = Date() + let start = end.addingTimeInterval(-3600 * 24) + + // 10 samples, one per minute, oldest first. Percent doubles as an ordering marker. + for index in 0..<10 { + context.insert(BatterySample(percent: index, timestamp: end.addingTimeInterval(Double(index - 10) * 60))) + } + try context.save() + + let capped = MetricsRepository.batterySamples(start: start, end: end, limit: 4, context: context) + + XCTAssertEqual(capped.count, 4) + XCTAssertEqual(capped.map(\.percent), [6, 7, 8, 9], "Expected the four most recent samples") + XCTAssertEqual(capped.map(\.timestamp), capped.map(\.timestamp).sorted(), + "Documented contract is oldest-first for a left-to-right axis") + } + + /// The uncapped path must be unaffected by the sort flip. + func testBatterySamplesStillReturnEverythingOldestFirstUnderTheLimit() throws { + let context = try TestSupport.makeContext() + let end = Date() + let start = end.addingTimeInterval(-3600 * 24) + + for index in 0..<5 { + context.insert(BatterySample(percent: index * 10, timestamp: end.addingTimeInterval(Double(index - 5) * 60))) + } + try context.save() + + let samples = MetricsRepository.batterySamples(start: start, end: end, context: context) + XCTAssertEqual(samples.map(\.percent), [0, 10, 20, 30, 40]) + } + + /// `latestSession` reads one row via `fetchLimit`; it must still be the newest one. + func testLatestSleepSessionIsTheMostRecentByStart() throws { + let context = try TestSupport.makeContext() + let now = Date() + + let older = SleepSession(date: now.addingTimeInterval(-86_400 * 2), + startAt: now.addingTimeInterval(-86_400 * 2), + endAt: now.addingTimeInterval(-86_400 * 2 + 3600 * 7), + totalMinutes: 420) + let newest = SleepSession(date: now, + startAt: now.addingTimeInterval(-3600 * 8), + endAt: now.addingTimeInterval(-3600), + totalMinutes: 420) + let middle = SleepSession(date: now.addingTimeInterval(-86_400), + startAt: now.addingTimeInterval(-86_400), + endAt: now.addingTimeInterval(-86_400 + 3600 * 7), + totalMinutes: 420) + // Inserted out of order so the result depends on the sort, not on insertion order. + [older, newest, middle].forEach { context.insert($0) } + try context.save() + + XCTAssertEqual(SleepRepository.latestSession(context: context)?.id, newest.id) + } + + func testLatestSleepSessionIsNilOnAnEmptyStore() throws { + let context = try TestSupport.makeContext() + XCTAssertNil(SleepRepository.latestSession(context: context)) + } +} From 4b1a2227c1bc5d3add6f9eee05bacf699c449b61 Mon Sep 17 00:00:00 2001 From: ak710 Date: Sat, 1 Aug 2026 23:40:10 -0400 Subject: [PATCH 4/5] Send the Gemini API key as a header, not in the URL GeminiClient interpolated the user's key into the query string as ?key=... A URL is the part of a request that gets written down -- URLSession logging, os_log, crash reports, proxies -- and headers are not. Gemini accepts x-goog-api-key, which is what the other three coach clients already do with their credentials. Interpolating also meant a key containing a URL-special character failed URL(string:) and surfaced as a misleading "could not build endpoint URL"; in a header it is just bytes. Adds header capture to the coach test stub so both properties can be asserted. Co-Authored-By: Claude Opus 5 --- PulseLoop/Coach/Gemini/GeminiClient.swift | 7 +++- PulseLoopTests/CoachTests.swift | 44 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/PulseLoop/Coach/Gemini/GeminiClient.swift b/PulseLoop/Coach/Gemini/GeminiClient.swift index 781bfa95..5a55224b 100644 --- a/PulseLoop/Coach/Gemini/GeminiClient.swift +++ b/PulseLoop/Coach/Gemini/GeminiClient.swift @@ -68,7 +68,7 @@ final class GeminiClient: ResponsesClient, @unchecked Sendable { let geminiBody = buildGeminiBody(tools: convertTools(tools), textFormat: textFormat) let geminiData = try JSONSerialization.data(withJSONObject: geminiBody) - let urlStr = "\(baseURL)/\(model):generateContent?key=\(apiKey)" + let urlStr = "\(baseURL)/\(model):generateContent" guard let url = URL(string: urlStr) else { throw ResponsesError.decoding("GeminiClient: could not build endpoint URL") } @@ -76,6 +76,11 @@ final class GeminiClient: ResponsesClient, @unchecked Sendable { var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("application/json", forHTTPHeaderField: "Content-Type") + // The key goes in a header, not `?key=` on the URL. URLs are the part of a request that gets + // written down — URLSession logging, os_log, crash reports, proxies — and this one is the + // user's own API key. Interpolating it also meant a key with a URL-special character failed + // `URL(string:)` and surfaced as a misleading "could not build endpoint URL". + request.setValue(apiKey, forHTTPHeaderField: "x-goog-api-key") request.httpBody = geminiData request.timeoutInterval = 60 diff --git a/PulseLoopTests/CoachTests.swift b/PulseLoopTests/CoachTests.swift index c0be364a..fcd2f750 100644 --- a/PulseLoopTests/CoachTests.swift +++ b/PulseLoopTests/CoachTests.swift @@ -21,10 +21,12 @@ final class StubURLProtocol: URLProtocol { nonisolated(unsafe) static var responseBody = Data() nonisolated(unsafe) static var statusCode = 200 nonisolated(unsafe) static var lastRequestURL: URL? + nonisolated(unsafe) static var lastRequestHeaders: [String: String] = [:] nonisolated(unsafe) static var lastRequestBody: Data? override class func canInit(with request: URLRequest) -> Bool { Self.lastRequestURL = request.url + Self.lastRequestHeaders = request.allHTTPHeaderFields ?? [:] Self.lastRequestBody = request.httpBody ?? request.httpBodyStream.flatMap { stream in stream.open(); defer { stream.close() } var data = Data() @@ -353,6 +355,48 @@ final class GeminiClientTests: XCTestCase { XCTAssertEqual(response.outputText, "hello") } + /// The key belongs in a header, not in `?key=`. A URL is the part of a request that gets written + /// down — URLSession logging, os_log, crash reports, proxies — and this one is the user's own + /// credential. + func testAPIKeyTravelsInAHeaderAndNeverInTheURL() async throws { + StubURLProtocol.statusCode = 200 + StubURLProtocol.responseBody = Data(#"{"candidates":[{"content":{"parts":[{"text":"ok"}]}}]}"#.utf8) + + let secret = "AIza-test-secret-key" + let client = GeminiClient(apiKey: secret, session: session()) + let body = try OpenAIRequestBuilder.data( + model: "gemini-2.5-flash", input: [], tools: [], textFormat: nil, + previousResponseId: nil, reasoningEffort: nil) + _ = try await client.send(requestBody: body) + + let url = StubURLProtocol.lastRequestURL?.absoluteString ?? "" + XCTAssertFalse(url.isEmpty, "expected the client to have issued a request") + XCTAssertFalse(url.contains(secret), "API key leaked into the URL: \(url)") + XCTAssertFalse(url.contains("key="), "API key leaked into the query string: \(url)") + + let keyHeader = StubURLProtocol.lastRequestHeaders.first { $0.key.lowercased() == "x-goog-api-key" }?.value + XCTAssertEqual(keyHeader, secret) + } + + /// A key containing URL-special characters used to fail `URL(string:)` and surface as a + /// misleading "could not build endpoint URL"; in a header it is just bytes. + func testKeyWithURLSpecialCharactersStillSends() async throws { + StubURLProtocol.statusCode = 200 + StubURLProtocol.responseBody = Data(#"{"candidates":[{"content":{"parts":[{"text":"ok"}]}}]}"#.utf8) + StubURLProtocol.lastRequestURL = nil + + let awkward = "abc def/+?&#%" + let client = GeminiClient(apiKey: awkward, session: session()) + let body = try OpenAIRequestBuilder.data( + model: "gemini-2.5-flash", input: [], tools: [], textFormat: nil, + previousResponseId: nil, reasoningEffort: nil) + _ = try await client.send(requestBody: body) + + XCTAssertNotNil(StubURLProtocol.lastRequestURL, "request should still be issued") + let keyHeader = StubURLProtocol.lastRequestHeaders.first { $0.key.lowercased() == "x-goog-api-key" }?.value + XCTAssertEqual(keyHeader, awkward) + } + /// Regression test: the user-selected model in the request body must drive the /// Gemini endpoint, not the client's hardcoded default. func testUsesModelFromRequestBody() async throws { From 9027fb69107ffa77483a58f8faf068df371e124f Mon Sep 17 00:00:00 2001 From: ak710 Date: Sat, 1 Aug 2026 23:40:10 -0400 Subject: [PATCH 5/5] Lint the widget extension, and stop rebuilding a formatter per row .swiftlint.yml listed PulseLoop, PulseLoopLiveActivity and PulseLoopTests, so PulseLoopWidgets (~1k lines across 6 files) was never checked by the CI lint job. Adding it surfaces two large_tuple warnings and no errors, so the job stays green. DiagnosticsExporter constructed a fresh ISO8601DateFormatter inside the map over every log line and every raw packet -- up to 700 allocations per export of an object that is expensive to build. One shared instance instead. Co-Authored-By: Claude Opus 5 --- .swiftlint.yml | 1 + PulseLoop/Diagnostics/DiagnosticsExporter.swift | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index c706f095..b9872bda 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -7,6 +7,7 @@ included: - PulseLoop - PulseLoopLiveActivity + - PulseLoopWidgets - PulseLoopTests excluded: diff --git a/PulseLoop/Diagnostics/DiagnosticsExporter.swift b/PulseLoop/Diagnostics/DiagnosticsExporter.swift index 735183f4..9fc6a481 100644 --- a/PulseLoop/Diagnostics/DiagnosticsExporter.swift +++ b/PulseLoop/Diagnostics/DiagnosticsExporter.swift @@ -9,10 +9,14 @@ import UIKit /// never leak protocol bytes. @MainActor enum DiagnosticsExporter { + /// One shared formatter: the log and packet maps below run this once per row, and constructing + /// an `ISO8601DateFormatter` is far more expensive than using one. + private static let iso = ISO8601DateFormatter() + /// Serialize a diagnostics report to pretty-printed JSON. static func exportJSON(context: ModelContext, maxLogs: Int = 500) -> String { var root: [String: Any] = [:] - root["generatedAt"] = ISO8601DateFormatter().string(from: Date()) + root["generatedAt"] = iso.string(from: Date()) root["app"] = appInfo() root["device"] = deviceInfo(context: context) root["logs"] = recentLogs(context: context, limit: maxLogs) @@ -30,7 +34,7 @@ enum DiagnosticsExporter { /// Write the report to a temporary file and return its URL (for a share sheet). static func exportFile(context: ModelContext) -> URL? { let json = exportJSON(context: context) - let stamp = ISO8601DateFormatter().string(from: Date()).replacingOccurrences(of: ":", with: "-") + let stamp = iso.string(from: Date()).replacingOccurrences(of: ":", with: "-") let url = FileManager.default.temporaryDirectory.appendingPathComponent("pulseloop-diagnostics-\(stamp).json") do { try json.data(using: .utf8)?.write(to: url) @@ -60,7 +64,7 @@ enum DiagnosticsExporter { info["wearableName"] = device.name info["capabilities"] = device.capabilities.csv info["firmware"] = device.firmwareVersion ?? "?" - info["lastSyncAt"] = device.lastSyncAt.map { ISO8601DateFormatter().string(from: $0) } ?? "" + info["lastSyncAt"] = device.lastSyncAt.map { iso.string(from: $0) } ?? "" } return info } @@ -71,7 +75,7 @@ enum DiagnosticsExporter { let logs = (try? context.fetch(descriptor)) ?? [] return logs.map { log in var row: [String: Any] = [ - "at": ISO8601DateFormatter().string(from: log.timestamp), + "at": iso.string(from: log.timestamp), "category": log.categoryRaw, "level": log.levelRaw, "message": log.message, @@ -88,7 +92,7 @@ enum DiagnosticsExporter { let packets = (try? context.fetch(descriptor)) ?? [] return packets.map { p in [ - "at": ISO8601DateFormatter().string(from: p.timestamp), + "at": iso.string(from: p.timestamp), "direction": p.directionRaw, "hex": p.hexPayload, "decoded": p.decodedKind ?? "",