perf: batch tRPC calls, cache device status, fix duplicate fetches#41
Merged
perf: batch tRPC calls, cache device status, fix duplicate fetches#41
Conversation
The iOS client was firing unbatched sequential HTTP requests where the web client uses tRPC's httpBatchLink to coalesce them. Cold launch also sat on a "Disconnected" screen for ~300ms waiting on the first fetch. Schedule writes were failing the server's max(100) cap once AI curves accumulated. - SleepypodCoreClient: add batchQuery helper mirroring @trpc/client's httpBatchLink. getDeviceStatus now batches 3 procs into 1 round trip (and drops a dead settings.getAll call). getServerStatus batches 6 health procs into 1. Query timeout 30s -> 8s, mutate 30s -> 15s. - updateSchedules: chunk delete/create arrays to <=100 per type so the server's z.array().max(100) validation stops rejecting AI-curve-sized batches. Surfaces tRPC error bodies on HTTP 4xx/5xx so silent fails become visible. - DeviceManager: persist deviceStatus to UserDefaults on every successful fetch; hydrate from cache in init so cold launch shows last-known values immediately instead of a blank state. - startPolling skips its first immediate tick when deviceStatus is already populated (from cache or a just-completed startConnection), eliminating a duplicate cold-start fetchStatus. - HealthScreen: remove redundant local fetchVitals that duplicated what MetricsManager.fetchAll already fetched, and the duplicate calibration call. Refresh now parallelizes fetchAll + calibration via async let. - TempScreen: .onAppear -> .task so fetchActiveCurve doesn't re-fire on every tab switch. Sort schedule setpoints by offset-from-bedtime so an overnight curve (22:00 -> 07:00) renders left-to-right as the chart expects (fixes "Now" line appearing ~20h into the future on a schedule with accumulated midnight entries). - Temp screen top bar shows "• just now" / "• 12s ago" via TimelineView, so you can tell cached from fresh data. Core-side followup filed as sleepypod/core#424: bump batchUpdate max(100) to max(1000) so we can drop the client-side chunking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds tRPC GET batching and bulk schedule mutations, introduces UserDefaults-backed device status caching with adjusted startup/polling order, refactors several views to use manager-provided state, and tweaks timeouts/response validation and sorting/display logic in temperature UI. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App/UI
participant DM as DeviceManager
participant Client as SleepypodCoreClient
participant Server as SleepypodCoreServer
participant UD as UserDefaults
UI->>DM: startup / user action
DM->>UD: loadCachedStatus()
alt cached status exists
UD-->>DM: cached DeviceStatus
DM-->>UI: populate UI (isConnected = true)
else no cache
DM->>Client: batchQuery([health,wifi,...]) / fetchStatus()
Client->>Server: HTTP batch GET (tRPC)
Server-->>Client: batch envelope (array of slots)
Client->>Client: parse slots -> per-slot Result<Data,Error>
Client-->>DM: decoded DeviceStatus(s)
DM->>UD: cacheStatus(status)
DM-->>UI: update UI (hasLiveFetched = true)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sleepypod/Networking/SleepypodCoreClient.swift`:
- Around line 32-39: The batch response decoding currently treats the
health.system slot as fatal; update the decoding in SleepypodCoreClient.swift so
that only the device.getStatus decode (from results[0].get()) remains throwing,
but decode the health.system slot (results[1].get()) the same way as wifi (use
try? decoder.decode(TRPCSystemHealth.self, from: ...)) and if it fails
substitute a sensible default TRPCSystemHealth metadata value; keep the wifi
decode as-is (try?) and leave batchQuery and device.getStatus logic unchanged so
polling continues even if health.system flakes.
In `@Sleepypod/Services/DeviceManager.swift`:
- Around line 37-43: The init() cold-launch hydration sets deviceStatus and
isConnected from cache which causes stale cached snapshots to look "connected"
after a failed first fetch; change the logic to track whether the current status
is cached vs live (e.g. add a hasLiveStatus or lastUpdated flag) when loading
cache in init() and set isConnected only if hasLiveStatus is true, then update
fetchStatus() to set hasLiveStatus = true and isConnected = true on a successful
network response and set hasLiveStatus = false and isConnected = false on a
failed fetch (even if deviceStatus is non‑nil) so cached snapshots no longer
cause the UI to render as connected indefinitely (refer to init(),
fetchStatus(), deviceStatus, isConnected and add hasLiveStatus/lastUpdated).
In `@Sleepypod/SleepypodApp.swift`:
- Around line 153-156: WelcomeScreen.onConnect currently calls
deviceManager.startPolling() before invoking startConnection(), causing a
duplicate fetchStatus() on manual connect; change the order in
WelcomeScreen.onConnect to await startConnection() first and then call
deviceManager.startPolling() so it matches the .task startup flow and lets
startPolling()/skipFirst logic skip the redundant initial fetchStatus() call.
In `@Sleepypod/Views/Data/HealthScreen.swift`:
- Around line 17-18: The view binds vitals directly to
MetricsManager.vitalsRecords which can update when fetchVitals completes before
other fetches finish, causing inconsistent UI; modify HealthScreen.swift to stop
reading metricsManager.vitalsRecords directly and instead snapshot the results
after the full fetchAll completes (e.g., after awaiting metricsTask in the
caller) or change MetricsManager.fetchAll() to publish atomically only after all
sub-fetches (fetchVitals, calibration, sleep-analysis) finish; specifically,
replace the computed property private var vitals: [VitalsRecord] {
metricsManager.vitalsRecords } with a local stored snapshot updated once after
await metricsTask (or adjust fetchAll() to commit a single published state) so
charts never observe mid-refresh intermediate vitalsRecords.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba49964f-e24c-4a2a-8cf9-f03e177f1251
📒 Files selected for processing (5)
Sleepypod/Networking/SleepypodCoreClient.swiftSleepypod/Services/DeviceManager.swiftSleepypod/SleepypodApp.swiftSleepypod/Views/Data/HealthScreen.swiftSleepypod/Views/Temp/TempScreen.swift
Comment on lines
+17
to
+18
| /// Vitals come from MetricsManager's fetchAll — don't re-fetch locally. | ||
| private var vitals: [VitalsRecord] { metricsManager.vitalsRecords } |
There was a problem hiding this comment.
Avoid binding the charts directly to MetricsManager.vitalsRecords mid-refresh.
MetricsManager.fetchAll() publishes vitalsRecords as soon as fetchVitals finishes in Sleepypod/Services/MetricsManager.swift, Lines 65-94, while calibration and the other metric fetches can still be in flight. With vitals now reading that observable directly, this view can briefly show fresh vitals against stale sleep-analysis/calibration state during a side or week change. Snapshot the refreshed records after await metricsTask, or make fetchAll() commit its results atomically.
Also applies to: 357-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Data/HealthScreen.swift` around lines 17 - 18, The view binds
vitals directly to MetricsManager.vitalsRecords which can update when
fetchVitals completes before other fetches finish, causing inconsistent UI;
modify HealthScreen.swift to stop reading metricsManager.vitalsRecords directly
and instead snapshot the results after the full fetchAll completes (e.g., after
awaiting metricsTask in the caller) or change MetricsManager.fetchAll() to
publish atomically only after all sub-fetches (fetchVitals, calibration,
sleep-analysis) finish; specifically, replace the computed property private var
vitals: [VitalsRecord] { metricsManager.vitalsRecords } with a local stored
snapshot updated once after await metricsTask (or adjust fetchAll() to commit a
single published state) so charts never observe mid-refresh intermediate
vitalsRecords.
- StatusManager / SleepCurve / LoadingView: drop pre-existing dead vars and the deprecated UIScreen.main reference that were failing CI under SWIFT_TREAT_WARNINGS_AS_ERRORS. Loading view now uses a fixed minHeight instead of UIScreen.main.bounds. - DeviceManager: add hasLiveFetched flag so cached cold-launch state doesn't claim isConnected=true when the pod is unreachable. Cache still hydrates the UI immediately, but disconnect surfaces correctly until a real fetch confirms reachability. - SleepypodCoreClient.getDeviceStatus: make health.system non-fatal (matches wifi). Polling no longer breaks if the health endpoint flakes. - SleepypodApp.WelcomeScreen.onConnect: connect first, then poll — matches the .task startup order so startPolling's skipFirst guard kicks in and avoids a duplicate initial fetch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the gap between iOS and web connect/load perf. The iOS client was firing unbatched sequential HTTP requests where web uses
@trpc/client'shttpBatchLink. Cold launch showed a blank "Disconnected" state for ~300ms waiting on the first fetch. Schedule writes were silently failing the server'smax(100)cap once AI-curve entries accumulated.Networking
batchQueryhelper mirroringhttpBatchLink's wire format (GET /api/trpc/a,b,c?batch=1&input=…). Per-slot success/failure so non-critical procs can fail independently.getDeviceStatus(hot path, 10s poll): 3 sequential procs → 1 batched round trip. Dropped an unusedsettings.getAllcall that was dead code.getServerStatus(Status tab): 7 sequential procs → 1 batched.validateResponselogs the tRPC error body on HTTP 4xx/5xx so we stop swallowing 400s silently.Schedule writes
updateScheduleschunks delete/create arrays to ≤100 per type per batch call. Server caps each atmax(100)(tracked on core side as schedules.batchUpdate: tight max(100) caps reject realistic payloads (AI curve × many days) core#424). Worst case "apply AI curve to 7 days" is now ~3 batch calls, each one transactional, instead of the old call being rejected entirely.Cold launch
DeviceManagerpersistsdeviceStatustoUserDefaultson every successful fetch (HTTP + WebSocket).initso Temp screen renders last-known values immediately — no more 300ms blank flash.startPollingskips its first immediate tick whendeviceStatus != nil, eliminating a redundant cold-start fetch that ran concurrently withstartConnection.Duplicate fetches
refresh()was firingmetricsManager.fetchAll(4 parallel) → then a localfetchVitalsthat duplicated whatfetchAllhad just fetched → then acheckCalibrationthat duplicatedfetchVitals's inline calibration. 6 requests → 5, now fully parallelized viaasync let. Removed the redundant local@State vitalsthat was shadow-storingmetricsManager.vitalsRecords..onAppear→.taskforfetchActiveCurve.onAppearre-fires on every tab switch;.taskfires once per view identity. Stops therunOnce.getActive+environment.getLatestAmbientLightdouble-fires visible in the request log.UI
Left • just now/Left • 12s agoviaTimelineView, auto-updating every 15s. Lets users distinguish cached from fresh data.power.on, defaulting to 22:00) instead of byHH:mmstring. String sort put03:00before22:00, making the banner anchor to the wake-side point and plotting 22:00/23:00 points ~20h into the chart future. Fixes "Now → 7am 48h out" visual bug.Test plan
xcodebuild buildfor simulator and device (nPhone)Last updatedindicator ticks correctlyFollowups
schedules.batchUpdatemax(100)tomax(1000)so we can drop the client-side chunking.batchQuerywould reduce that, but it's already parallel (wall time =max(t1..t5)), so lower priority than the cold-launch wins.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / Improvements