fix: seed bed temp trend from tRPC historical data#33
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds BedTempReading model, protocol + client + stub implementations, a BedTempTrendView UI, SensorStreamService seeding to fetch and populate historical left/right temperature histories (max 120), chart Y-axis tightening, and project/test updates to wire the new view and APIs. Changes
Sequence DiagramsequenceDiagram
participant Service as SensorStreamService
participant Client as SleepypodCoreClient
participant tRPC as tRPC Backend
participant WS as WebSocket Stream
Service->>Service: connect()
Service->>Service: check tempHistorySeeded
alt not seeded
Service->>Client: getBedTempHistory(start, end, limit, unit)
Client->>Client: format dates (ISO-8601)
Client->>tRPC: query "environment.getBedTemp"
tRPC-->>Client: [BedTempReading]
Client-->>Service: return readings
Service->>Service: reverse to chronological order
Service->>Service: append to left/right histories
Service->>Service: trim to maxTempHistory (120)
Service->>Service: set tempHistorySeeded = true
end
Service->>WS: listen for bedTemp2 frames
WS-->>Service: live bedTemp2 frame
Service->>Service: mark tempHistorySeeded = true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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.
🧹 Nitpick comments (2)
Sleepypod/Models/BetaModels.swift (1)
88-90: Consider caching theISO8601DateFormatterinstance.Creating a new
ISO8601DateFormatteron each access is expensive. InseedTempHistory(), this computed property is called for each reading in a loop (up to 120 times), creating unnecessary allocations.♻️ Suggested refactor using a static formatter
+private let iso8601Formatter: ISO8601DateFormatter = { + let formatter = ISO8601DateFormatter() + return formatter +}() + struct BedTempReading: Decodable, Sendable { let timestamp: String // ISO8601 let leftCenterTemp: Double? let leftInnerTemp: Double? let rightCenterTemp: Double? let rightInnerTemp: Double? var leftF: Float? { if let t = leftCenterTemp ?? leftInnerTemp { return Float(t) } return nil } var rightF: Float? { if let t = rightCenterTemp ?? rightInnerTemp { return Float(t) } return nil } var date: Date? { - ISO8601DateFormatter().date(from: timestamp) + iso8601Formatter.date(from: timestamp) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Models/BetaModels.swift` around lines 88 - 90, The computed property date in BetaModels.swift repeatedly constructs an ISO8601DateFormatter which is expensive when seedTempHistory() iterates many readings; replace the per-access formatter with a shared cached instance (e.g., a static or file-level ISO8601DateFormatter) and have the date getter use that singleton formatter to parse timestamp, ensuring thread-safety if needed.Sleepypod/Services/SensorStreamService.swift (1)
144-149: Consider storing the seeding task reference for proper cancellation.The spawned
Taskreference is not stored, so it cannot be cancelled indisconnect(). Ifconnect()is called rapidly multiple times before seeding completes, multiple API calls may be made (though the double-guard pattern inseedTempHistory()correctly prevents duplicate data insertion).♻️ Suggested refactor to track the seeding task
private var reconnectTask: Task<Void, Never>? private var demoTask: Task<Void, Never>? + private var seedTask: Task<Void, Never>? private var frameCount = 0In
connect():// Seed temp trend from tRPC historical data (once per session) if !tempHistorySeeded { - Task { await self.seedTempHistory() } + seedTask = Task { await self.seedTempHistory() } }In
disconnect():func disconnect() { demoTask?.cancel(); demoTask = nil + seedTask?.cancel(); seedTask = nil receiveTask?.cancel(); receiveTask = nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Services/SensorStreamService.swift` around lines 144 - 149, The seeding Task spawned in connect() isn't stored so it can't be cancelled; add a stored property (e.g. tempHistorySeedTask: Task<Void, Never>?) to SensorStreamService, only create the Task if tempHistorySeedTask is nil and tempHistorySeeded is false, assign the Task when launching Task { await self.seedTempHistory() }, and in disconnect() call tempHistorySeedTask?.cancel() and set it to nil; also clear tempHistorySeedTask when seedTempHistory() completes so subsequent connects can create a new task if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sleepypod/Models/BetaModels.swift`:
- Around line 88-90: The computed property date in BetaModels.swift repeatedly
constructs an ISO8601DateFormatter which is expensive when seedTempHistory()
iterates many readings; replace the per-access formatter with a shared cached
instance (e.g., a static or file-level ISO8601DateFormatter) and have the date
getter use that singleton formatter to parse timestamp, ensuring thread-safety
if needed.
In `@Sleepypod/Services/SensorStreamService.swift`:
- Around line 144-149: The seeding Task spawned in connect() isn't stored so it
can't be cancelled; add a stored property (e.g. tempHistorySeedTask: Task<Void,
Never>?) to SensorStreamService, only create the Task if tempHistorySeedTask is
nil and tempHistorySeeded is false, assign the Task when launching Task { await
self.seedTempHistory() }, and in disconnect() call tempHistorySeedTask?.cancel()
and set it to nil; also clear tempHistorySeedTask when seedTempHistory()
completes so subsequent connects can create a new task if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 764f3cad-777b-411a-b629-e858db9c3b51
📒 Files selected for processing (7)
Sleepypod/Models/BetaModels.swiftSleepypod/Networking/FreeSleepClient.swiftSleepypod/Networking/MockClient.swiftSleepypod/Networking/SleepypodCoreClient.swiftSleepypod/Networking/SleepypodProtocol.swiftSleepypod/Services/SensorStreamService.swiftSleepypodTests/MockAPIClient.swift
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@metadata/app_store_submission.md`:
- Line 32: Several fenced code blocks are missing language identifiers which
triggers MD040; edit each opening triple-backtick fence that wraps the blocks
containing "sleepypod — local control for your Eight Sleep Pod", the keywords
line starting "sleep,temperature,eight sleep,...", the numbered demo
instructions starting "1. On first launch, tap 'Explore Demo'", and the release
notes starting "Initial release — local control for your Eight Sleep Pod." to
include a language tag (e.g., ```text) immediately after the backticks so every
fenced code block has a language identifier.
- Around line 126-130: The file currently contains reviewer PII and a
placeholder in the table rows labeled "Contact First Name", "Contact Last Name",
"Contact Email", and "Contact Phone" (and the "Demo Account" entry); replace
these hardcoded values with template variables (e.g., {{contact_first_name}},
{{contact_last_name}}, {{contact_email}}, {{contact_phone}} or similar) and
remove any real personal info so the repo only contains placeholders; ensure the
template is used and real reviewer contact details are injected from an
out-of-repo secrets/config mechanism at submission time rather than committed in
the repository.
In `@Sleepypod/Views/Sensor/BedSensorScreen.swift`:
- Around line 312-317: The tempYDomain getter is vulnerable to single-sample
outliers from sensor.leftTempHistory/sensor.rightTempHistory flattening the
trend; change it to compute robust bounds (e.g., drop extreme tails or use
5th/95th percentiles or a trimmed min/max) and then enforce a minimum span
(e.g., at least 2°F) and/or clamp to reasonable absolute limits before returning
the ClosedRange; apply the same fix to the other identical occurrence that uses
these histories so the Y-axis remains tight but resistant to anomalies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3962f159-7910-4c93-9842-0511291e161a
📒 Files selected for processing (2)
Sleepypod/Views/Sensor/BedSensorScreen.swiftmetadata/app_store_submission.md
|
|
||
| ## Description | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
These fences trigger MD040 and will keep lint noisy until fixed.
💡 Suggested patch
-```
+```text
sleepypod — local control for your Eight Sleep Pod
...
-```
+```
-```
+```text
sleep,temperature,eight sleep,pod,mattress,sleep tracking,heart rate,HRV,smart bed,local control
-```
+```
-```
+```text
1. On first launch, tap "Explore Demo" to use the app with simulated data
...
To exit demo: Settings gear (top-right) → "Connect to Real Pod"
-```
+```
-```
+```text
Initial release — local control for your Eight Sleep Pod.
...
• Demo mode for exploring without hardware
-```
+```Also applies to: 74-74, 105-105, 149-149
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@metadata/app_store_submission.md` at line 32, Several fenced code blocks are
missing language identifiers which triggers MD040; edit each opening
triple-backtick fence that wraps the blocks containing "sleepypod — local
control for your Eight Sleep Pod", the keywords line starting
"sleep,temperature,eight sleep,...", the numbered demo instructions starting "1.
On first launch, tap 'Explore Demo'", and the release notes starting "Initial
release — local control for your Eight Sleep Pod." to include a language tag
(e.g., ```text) immediately after the backticks so every fenced code block has a
language identifier.
Comment on lines
+312
to
+317
| /// Tight Y-axis domain (±1°F around data range) so left/right lines are visually distinct. | ||
| private var tempYDomain: ClosedRange<Float> { | ||
| let allTemps = sensor.leftTempHistory.map(\.1) + sensor.rightTempHistory.map(\.1) | ||
| guard let lo = allTemps.min(), let hi = allTemps.max() else { return 70...80 } | ||
| return (lo - 1)...(hi + 1) | ||
| } |
There was a problem hiding this comment.
Harden tempYDomain against outliers so the trend doesn’t flatten again.
Because history values are raw (see Sleepypod/Models/BetaModels.swift:71-86), one anomalous sample can dominate min/max and defeat the “tight axis” goal.
💡 Suggested patch
/// Tight Y-axis domain (±1°F around data range) so left/right lines are visually distinct.
private var tempYDomain: ClosedRange<Float> {
- let allTemps = sensor.leftTempHistory.map(\.1) + sensor.rightTempHistory.map(\.1)
- guard let lo = allTemps.min(), let hi = allTemps.max() else { return 70...80 }
- return (lo - 1)...(hi + 1)
+ let plausibleRange: ClosedRange<Float> = 50...120
+ let allTemps = (sensor.leftTempHistory.map(\.1) + sensor.rightTempHistory.map(\.1))
+ .filter { $0.isFinite && plausibleRange.contains($0) }
+ guard let lo = allTemps.min(), let hi = allTemps.max() else { return 70...80 }
+
+ let paddedLo = lo - 1
+ let paddedHi = hi + 1
+ let minSpan: Float = 4
+ let span = max(paddedHi - paddedLo, minSpan)
+ let mid = (paddedLo + paddedHi) / 2
+ return (mid - span / 2)...(mid + span / 2)
}Also applies to: 352-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Sensor/BedSensorScreen.swift` around lines 312 - 317, The
tempYDomain getter is vulnerable to single-sample outliers from
sensor.leftTempHistory/sensor.rightTempHistory flattening the trend; change it
to compute robust bounds (e.g., drop extreme tails or use 5th/95th percentiles
or a trimmed min/max) and then enforce a minimum span (e.g., at least 2°F)
and/or clamp to reasonable absolute limits before returning the ClosedRange;
apply the same fix to the other identical occurrence that uses these histories
so the Y-axis remains tight but resistant to anomalies.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sleepypod/Views/Sensor/BedTempTrendView.swift (1)
225-227: Avoid swallowing fetch failures silently.The empty catch makes operational debugging difficult. Emit a lightweight log (or telemetry event) for non-cancellation failures.
🛠️ Suggested fix
- } catch { - // Non-fatal — chart stays empty + } catch is CancellationError { + // Expected when range changes quickly + } catch { + Logger.sensor.error("BedTempTrendView history fetch failed: \(error.localizedDescription)") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Sensor/BedTempTrendView.swift` around lines 225 - 227, In BedTempTrendView replace the empty catch block that silently swallows errors with a lightweight log/telemetry emission: inside the catch in the async fetch (the catch in BedTempTrendView where chart data is parsed/fetched), check for CancellationError (e.g., if (error as? CancellationError) { return }) and otherwise call your logger/telemetry (e.g., Logger.debug/Logger.error or a telemetry.track) with a short message and the error to record non-fatal failures; ensure you reference the same catch block and keep the message concise to avoid noisy logs.
🤖 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/Views/Sensor/BedTempTrendView.swift`:
- Around line 165-167: The async fetch can return after `range` has changed, so
within the `.task(id: range)`/`fetchData()` flow capture the current `range`
into a local (e.g., let currentRange = range) before calling
`getBedTempHistory`, then after the network call and before assigning
`self.points` verify that `currentRange == range` (and optionally check
Task.isCancelled) and only then update `points`; reference the `.task(id:
range)` modifier, the `fetchData()` function, the `getBedTempHistory` call, and
the `points`/`range` state when making this guard.
---
Nitpick comments:
In `@Sleepypod/Views/Sensor/BedTempTrendView.swift`:
- Around line 225-227: In BedTempTrendView replace the empty catch block that
silently swallows errors with a lightweight log/telemetry emission: inside the
catch in the async fetch (the catch in BedTempTrendView where chart data is
parsed/fetched), check for CancellationError (e.g., if (error as?
CancellationError) { return }) and otherwise call your logger/telemetry (e.g.,
Logger.debug/Logger.error or a telemetry.track) with a short message and the
error to record non-fatal failures; ensure you reference the same catch block
and keep the message concise to avoid noisy logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a799680b-83b9-4668-b2bf-2966ba5f46eb
📒 Files selected for processing (4)
Sleepypod.xcodeproj/project.pbxprojSleepypod/Models/BetaModels.swiftSleepypod/Views/Sensor/BedSensorScreen.swiftSleepypod/Views/Sensor/BedTempTrendView.swift
✅ Files skipped from review due to trivial changes (1)
- Sleepypod.xcodeproj/project.pbxproj
🚧 Files skipped from review as they are similar to previous changes (2)
- Sleepypod/Views/Sensor/BedSensorScreen.swift
- Sleepypod/Models/BetaModels.swift
- New BedTempTrendView: three lines (left/right/ambient), time range selector (1H/6H/12H/24H), legend, and summary stats row - Fetch historical bed temp from tRPC environment.getBedTemp endpoint - Add BedTempReading model, getBedTempHistory to protocol + all clients - Fix ISO8601 date parsing to handle fractional seconds - Guard against stale async responses on rapid range switching - Remove old WS-only tempTrendCard (replaced by tRPC-backed chart) - Matches web core's BED TEMPERATURE card
ng
force-pushed
the
fix/bed-temp-trend-history
branch
from
e200d02 to
eb3b4db
Compare
4 tasks
ng
added a commit
that referenced
this pull request
Mar 29, 2026
* feat: bed temperature trend chart with historical data (#33) - New BedTempTrendView: three lines (left/right/ambient), time range selector (1H/6H/12H/24H), legend, and summary stats row - Fetch historical bed temp from tRPC environment.getBedTemp endpoint - Add BedTempReading model, getBedTempHistory to protocol + all clients - Fix ISO8601 date parsing to handle fractional seconds - Guard against stale async responses on rapid range switching - Remove old WS-only tempTrendCard (replaced by tRPC-backed chart) - Matches web core's BED TEMPERATURE card * feat: Use Now split button for run-once curves (#251) (#34) * feat: bed temperature trend chart with historical data - New BedTempTrendView: three lines (left/right/ambient), time range selector (1H/6H/12H/24H), legend, and summary stats row - Fetch historical bed temp from tRPC environment.getBedTemp endpoint - Add BedTempReading model, getBedTempHistory to protocol + all clients - Fix ISO8601 date parsing to handle fractional seconds - Guard against stale async responses on rapid range switching - Remove old WS-only tempTrendCard (replaced by tRPC-backed chart) - Matches web core's BED TEMPERATURE card * feat: "Use Now" split button for one-off curve application (#251) Split the "Apply to Schedule" button into two actions: - Apply to Schedule: existing behavior, persists as recurring weekly entries - Use Now: applies curve from now until wake time via runOnce.start API Adds RunOnceSession model, networking methods (start/getActive/cancel) to protocol and all client implementations. * feat: active curve banner, AI curve Use Now, polish - Active curve banner on Temp/Schedule/Status screens with chart, NOW marker, and Stop button - Stop Curve powers off the heater - Fade in/out animations on banner - AI curve wizard: Save | Apply | Use Now split bar - Use Now time-shifts curve proportionally (bedtime=now, wake preserved) - Switches to Temp tab after Use Now - Chart: single accent line + phase-colored dots, centered legend - NOW line uses minute-offset interpolation for accurate positioning - X-axis: 4 evenly spaced ticks, rotated labels, bottom padding * feat: AI prompt returns nowCurve for properly shaped start-now variant The AI prompt now asks for two curves: - "points": reusable template curve (bedtime to wake, as before) - "nowCurve": properly shaped curve starting from current time to wake When the user taps "Use Now", the AI's nowCurve is used if available (properly adapted phases for shorter duration). Falls back to proportional time-shifting if nowCurve is missing. Also: chart line uses single accent color, dots show phase colors, legend centered. * fix: fetch active curve on every Temp tab appearance, not just first load * feat: show tonight's active curve from any source on Temp page The Temp page now always shows the active curve — whether it came from a "Use Now" run-once session or today's recurring schedule: 1. Run-once active → show with "ACTIVE CURVE" header + Stop button 2. Today's recurring schedule has set points → show with "TONIGHT'S SCHEDULE" header (no Stop button — edit on Schedule page) 3. Neither → show manual - power + controls Adds ScheduleManager to TempScreen environment, unified ActiveCurve model, and isSchedule flag on RunOnceActiveBanner. * fix: prevent disconnect/reconnect when adjusting temperature The polling loop and failure handler were calling fetchStatus (HTTP to hardware) while a mutation was in flight, causing hardware contention and temporary disconnects. - Add isSendingMutation flag to block polling during active mutations - Only call fetchStatus on failure when WS isn't providing updates (the event bus already pushes the correct state after mutations) - Trust the WS event bus as the primary state source * fix: eliminate fetchStatus calls during mutations, add 2s cooldown * fix: only show disconnected view when never connected, not on transient failures * fix: sync side selector when Use Now switches to Temp tab * fix: auto-retry mDNS discovery after Local Network permission prompt * fix: add eight-pod.local hostname fallback when mDNS multicast fails * fix: address all PR #34 review feedback - Hide "Use Now" button when backend is FreeSleep (not supported) - Use displayed curve for "Use Now" (custom or generated, not always regenerated) - Only show "Started!" after all API calls succeed - BedTempTrendView respects user's temperature format (C/F) - Clear stale chart data when range switch fails - isCancelling resets after 5s timeout if banner isn't dismissed - Now marker clamped to chart domain (no off-screen rendering) - Warn on malformed nowCurve (log, don't block main curve) * fix: remove processingStatus contract test (endpoint removed in core)
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
6 tasks
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
environment.getBedTemptRPC endpoint on connect, then appends live WS frames as they arriveTempTrendChartbehavior (seed from tRPC, append from WS)Changes
BedTempReadingmodel withleftF/rightFcomputed propertiesgetBedTempHistory()to protocol + all client implementationsleftTempHistory/rightTempHistoryfrom tRPC on WS connectmaxTempHistoryfrom 60 to 120 to match web coreTest plan
Summary by CodeRabbit
New Features
Improvements
Documentation