Skip to content

Fix date-key, 24-hour clock, query truncation, and API-key-in-URL bugs - #106

Open
ak710 wants to merge 5 commits into
saksham2001:mainfrom
ak710:fix/date-locale-and-query-efficiency
Open

Fix date-key, 24-hour clock, query truncation, and API-key-in-URL bugs#106
ak710 wants to merge 5 commits into
saksham2001:mainfrom
ak710:fix/date-locale-and-query-efficiency

Conversation

@ak710

@ak710 ak710 commented Aug 2, 2026

Copy link
Copy Markdown

Summary

Five independent bug fixes found while reading through the app. None of them are visible on a default US-locale device, which is roughly why they survived. Each commit stands alone and can be dropped or taken separately.

  1. Date keys followed the device's calendar. DateFormatter inherits 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 / 8-08-01. That is right for display and wrong for the seven strings it affected, which are identifiers: coach summary scope keys, the notification dedupe key, export/share-card filenames, and the date arguments the coach emits and parses back. A key that moves with the setting stops matching stored keys, stops sorting chronologically, and stops being a date the model can read back. Adds DateFormatter.stableKey, pinning en_US_POSIX + Gregorian — the combination BatteryAlertMonitor already used for its own dedupe key.

  2. Six display sites hard-coded 12-hour time. "h:mm a" forces "9:30 PM" on users who have turned on 24-Hour Time, and in locales where 24-hour is the norm: sleep stage chart axis, sleep bed/wake times, workout summary header, meal detail, share card. Now locale templates ("jmm"9:30 PM or 21:30). Month/day patterns move to templates too, which also fixes field order per locale (Aug 1 vs 1 Aug).

  3. Two windowed reads misbehaved. SleepRepository.latestSession sorted the whole SleepSession table to take .first with no fetchLimit, unlike every sibling in the file. MetricsRepository.batterySamples sorted forward then applied fetchLimit, so a window over the cap returned the oldest rows and dropped the newest — the opposite of what the drainage chart is for.

  4. The Gemini API key travelled in the URL. ?key=... puts the user's credential in the part of a request that gets written down (URLSession logging, os_log, crash reports, proxies). Moved to the x-goog-api-key header, matching what the other three coach clients do. Interpolating also meant a key with a URL-special character failed URL(string:) and surfaced as a misleading "could not build endpoint URL".

  5. PulseLoopWidgets was never linted, and DiagnosticsExporter built a fresh ISO8601DateFormatter inside the map over every log line and packet (up to ~700 per export).

Related issues

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 📟 New / improved wearable support (BLE driver layer)
  • 🤖 Coach / LLM change (tools, prompts, orchestration)
  • 🎨 UI / DesignSystem change
  • 🧹 Refactor / chore (no behavior change)
  • 📝 Docs only
  • ⚠️ Breaking change (existing data, settings, or APIs change)

How was this tested?

  • Added / updated unit tests (PulseLoopTests)
  • Ran the test suite locally (⌘U in Xcode)
  • Tested on a physical device with a real ring — model:
  • Tested with demo data (-seedDemo YES, no hardware)
  • N/A (docs / non-code change)

885 tests pass, swiftlint reports 0 errors (the widget files it now covers add 2 large_tuple warnings and no errors).

12 new tests across DateFormattingTests, RepositoryFetchLimitTests, and GeminiClientTests. Each was checked against the unfixed code first and fails there — e.g. reverting the two behavioural changes turns 5 of them red — so they are regression guards rather than restatements of current behaviour.

Not exercised on hardware: none of this touches the BLE layer. The riskiest change for a device check is #2, since it alters visible strings — the sleep chart axis and workout summary header are the places to look.

Privacy & data

  • This change does not send health data off-device without explicit user action.
  • No secrets, API keys, or personal data are committed.

Fix #4 strictly reduces exposure of the user's own API key.

Checklist

  • My code follows the project's style (SwiftLint passes).
  • I ran the tests and they pass.
  • I updated docs / README where relevant.
  • I read the Contributing guide.

Notes for review

CONTRIBUTING asks for one logical change per PR — these are five, kept as five separate commits for that reason. Happy to split into separate PRs if you'd prefer.

A few smaller things I noticed and deliberately left alone, since each needs a judgement call that is yours:

  • CoachDataAccess.dayBounds returns next-midnight as end while the repository predicates use <= end, so a reading at exactly 00:00:00 is counted in both adjacent days. The fix is one character, but it shifts behaviour for every caller of those shared predicates.
  • CoachDataAccess's day/time helpers still build a formatter per call inside maps over rows. Caching them means either a stale time zone or a per-call reset, so it wanted a decision rather than a drive-by.
  • ForEach(sessions.indices, id: \.self) in SleepView and RecordSummaryViews uses index-as-identity over data that can reorder.

ak710 and others added 5 commits August 1, 2026 23:39
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
.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 <noreply@anthropic.com>
@ak710
ak710 requested a review from saksham2001 as a code owner August 2, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for Colmi R11

1 participant