Skip to content

Watch PR review fixes#598

Merged
bjorkert merged 2 commits intoapple-watchfrom
watch_review_fixes
Apr 10, 2026
Merged

Watch PR review fixes#598
bjorkert merged 2 commits intoapple-watchfrom
watch_review_fixes

Conversation

@bjorkert
Copy link
Copy Markdown
Contributor

@bjorkert bjorkert commented Apr 9, 2026

Addresses the pre-merge review items on #594.

Blockers

  • Guard LoopFollowDebugCorner behind #if DEBUG. The diagnostic complication (snapshot updatedAt vs. ClockKit timeline-entry time) was listed in getComplicationDescriptors unconditionally, so end users would see "LoopFollow Debug" in the complication picker. The descriptor, the two template builders (graphicCornerDebugTemplate, the stale/placeholder debug branches), and ComplicationID.debugCorner itself are now all wrapped in #if DEBUG.
  • Remove dead LAAppGroupSettings.setWatchSlots(_:) / watchSlots(). The fixed-4-slot variant was superseded by watchSelectedSlots and had no callers. Deleted along with the watch.slots key.

File layout

  • Move WatchConnectivityManager.swift into LoopFollow/LiveActivity/. It previously sat at the repo root in the top-level Xcode group. The file is still compiled into the main iOS app only; the rename just puts it next to the rest of the snapshot pipeline (GlucoseSnapshotStore, LiveActivityManager, GlucoseSnapshotBuilder, LAAppGroupSettings). project.pbxproj updated to reference the new group membership.
  • Drop duplicate file header. WatchConnectivityManager.swift had both the standard // LoopFollow / // WatchConnectivityManager.swift header and a second // WatchConnectivityManager.swift / // Philippe Achkar / // 2026-03-10 block. Collapsed to the single standard header the rest of the codebase uses.

Thread-safety

  • Serialize WatchConnectivityManager.lastWatchAckTimestamp. send(snapshot:) is documented as safe to call from any thread, and WCSession delegate callbacks arrive on an arbitrary background queue, so the bare TimeInterval was read/written without synchronization. Reads and writes now go through a dedicated stateQueue (DispatchQueue(label: "com.loopfollow.WatchConnectivityManager.state")). Exposed as a computed property so callers don't change.
  • Assert main-thread access for WatchSessionReceiver.cacheComplication. The caller contract was only a doc comment. Now enforced with dispatchPrecondition(condition: .onQueue(.main)) and the invariant is repeated on cachedComplications.

Nits

  • Drop the no-op JSONEncoder.dateEncodingStrategy = .iso8601 / JSONDecoder.dateDecodingStrategy = .iso8601 lines in GlucoseSnapshotStore, WatchConnectivityManager, WatchSessionReceiver, and LoopFollowWatchApp.decodeContextSnapshot. GlucoseSnapshot has a custom encoder/decoder that writes and reads updatedAt as a Double directly, so the strategy was never consulted.
  • ComplicationEntryBuilder: if let _ = snapshot.projectedif snapshot.projected != nil.
  • ContentView.SlotSelectionView: add a one-line comment explaining why .delta and .projectedBG are filtered out of the picker (they're always shown on the glucose page).

Also

  • Remove CLAUDE.md.

Build: xcodebuild -workspace LoopFollow.xcworkspace -scheme LoopFollow -destination 'platform=iOS Simulator,name=iPhone 17' buildBUILD SUCCEEDED.

bjorkert added 2 commits April 9, 2026 20:40
- Guard LoopFollowDebugCorner descriptor, template, and placeholder
  branches with #if DEBUG so the diagnostic complication does not ship
  to end users.
- Remove unused LAAppGroupSettings.setWatchSlots / watchSlots (the
  fixed-4-slot variant superseded by watchSelectedSlots).
- Move WatchConnectivityManager.swift from the repo root into
  LoopFollow/LiveActivity/ next to the rest of the snapshot pipeline
  and drop the duplicate file header.
- Serialize WatchConnectivityManager.lastWatchAckTimestamp access
  through a dedicated state queue. send(snapshot:) may run on any
  thread and WCSession delegate callbacks arrive on a background
  queue, so the bare TimeInterval was racy.
- Assert main-thread access for WatchSessionReceiver.cacheComplication
  and document the invariant on cachedComplications.
- Drop the no-op JSONEncoder/JSONDecoder iso8601 date strategies —
  GlucoseSnapshot has a custom encoder/decoder that writes/reads
  updatedAt as a Double, so the strategy was never consulted.
- Nit: replace `if let _ = snapshot.projected` with
  `if snapshot.projected != nil`.
- Nit: comment why SlotSelectionView excludes .delta / .projectedBG.
- Remove CLAUDE.md.
cacheComplication is called from 3 sites: getCurrentTimelineEntry,
getTimelineEndDate (ClockKit callbacks — not documented as strictly
main-thread), and reloadComplicationsOnMainThread. The precondition
could crash if ClockKit ever delivers on a background queue.
@bjorkert bjorkert merged commit cb286c1 into apple-watch Apr 10, 2026
@bjorkert bjorkert deleted the watch_review_fixes branch April 10, 2026 08:28
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.

1 participant