Schedule loading, clear, and Health screen overhaul#16
Conversation
- On appear + day change: loads bedtime from power schedule, wake from alarm, min/max from temperature set points - Tapping a different day updates the curve to reflect saved data - If no schedule exists, defaults remain (22:00-07:00, 68-86°F) - Apply to Schedule now works end-to-end (core#199 was fixed)
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a schedule-clear UI and backend flow to ScheduleScreen; syncs SmartCurveView state from schedules on appear/selection changes; reworks updateSchedules to handle per-side, per-day delete/create logic; significantly expands SleepStagesTimelineView and adjusts HealthScreen, SideSelector/TempScreen layout tweaks, and Xcode project signing info. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ScheduleScreen as "ScheduleScreen (UI)"
participant Client as "SleepypodCoreClient"
participant Server as "API/Server"
participant Manager as "scheduleManager"
User->>ScheduleScreen: Tap "Clear Schedule"
ScheduleScreen->>User: Present confirmation alert
User->>ScheduleScreen: Confirm clear
ScheduleScreen->>Manager: update local selection -> prepare cleared DailySchedule
ScheduleScreen->>Client: clearSchedule() -> updateSchedules(per-side, per-day)
Client->>Server: DELETE / POST per-side per-day changes
Server-->>Client: 200 OK
Client-->>Manager: refresh schedules / update state
Manager-->>ScheduleScreen: selectedDay/state updated
ScheduleScreen->>User: Haptic + UI reflects cleared days
sequenceDiagram
participant View as "SmartCurveView"
participant Manager as "scheduleManager"
participant UIState as "Local view state (bedtime,wake,temps)"
View->>Manager: onAppear / onChange(selectedDay or schedules)
Manager-->>View: currentDailySchedule
View->>UIState: loadFromSchedule() -> set bedtime, wakeTime, minTemp, maxTemp
UIState-->>View: UI updates controls/curve
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)
📝 Coding Plan
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 |
- 'Clear Schedule' button appears when phases exist for selected days - Confirmation alert: 'Clear Selected Days' / Cancel - Clears temperature, power, and alarm for all selected days + side - Deletes via updateSchedules (delete-all-then-recreate)
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 `@Sleepypod/Views/Schedule/ScheduleScreen.swift`:
- Around line 113-120: The code optimistically sets scheduleManager.schedules
before persisting with APIBackend.current.createClient() and
api.updateSchedules(schedules); if that call fails you must revert
scheduleManager.schedules to the previous value and surface an error to the user
instead of silently logging it. Capture the previous schedules (e.g., let
previous = scheduleManager.schedules) before mutating, await
api.updateSchedules, and on catch restore scheduleManager.schedules = previous,
avoid triggering Haptics.heavy() on failure, and present a user-visible error
(alert/toast) in addition to Log.general.error("Failed to clear schedule:
\(error)").
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 521-523: The loadFromSchedule() function currently early-returns
when scheduleManager.currentDailySchedule is nil and leaves previous state in
place; change it to apply default values when no schedule exists by removing the
early return and using the app's default min/max/temperatures (or explicit
defaults) whenever currentDailySchedule is nil. Also, when daily.temperatures is
empty (the block around the temperatures check at lines ~546-550), ensure you
reset minTemp and maxTemp to those same defaults instead of leaving stale
values; update the logic in loadFromSchedule() to compute min/max only when
temperatures.nonEmpty and otherwise assign defaults, referencing
scheduleManager.currentDailySchedule, daily.temperatures, and the
minTemp/maxTemp properties in that method.
- Around line 173-174: The view only calls loadFromSchedule() on appear and when
scheduleManager.selectedDay changes, so it misses updates when schedules finish
loading; update SmartCurveView to also trigger loadFromSchedule() when the
schedule data changes (e.g., add an .onChange(of: scheduleManager.schedules) or
.onChange(of: scheduleManager.isLoaded / isLoading) modifier) so that when the
ScheduleManager finishes loading its schedules it will call loadFromSchedule()
and populate the view.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bb60b42-14b3-4003-ac8a-b992a39a19e0
📒 Files selected for processing (2)
Sleepypod/Views/Schedule/ScheduleScreen.swiftSleepypod/Views/Schedule/SmartCurveView.swift
| scheduleManager.schedules = schedules | ||
| do { | ||
| let api = APIBackend.current.createClient() | ||
| scheduleManager.schedules = try await api.updateSchedules(schedules) | ||
| Haptics.heavy() | ||
| } catch { | ||
| Log.general.error("Failed to clear schedule: \(error)") | ||
| } |
There was a problem hiding this comment.
Failed clear leaves local state out of sync with backend.
At Line 113, state is mutated optimistically, but on failure (Line 118-120) there is no rollback or user-visible error. Users can see a cleared schedule that was never persisted.
Proposed fix
- scheduleManager.schedules = schedules
+ let previousSchedules = scheduleManager.schedules
+ scheduleManager.schedules = schedules
do {
let api = APIBackend.current.createClient()
scheduleManager.schedules = try await api.updateSchedules(schedules)
Haptics.heavy()
} catch {
+ scheduleManager.schedules = previousSchedules
Log.general.error("Failed to clear schedule: \(error)")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| scheduleManager.schedules = schedules | |
| do { | |
| let api = APIBackend.current.createClient() | |
| scheduleManager.schedules = try await api.updateSchedules(schedules) | |
| Haptics.heavy() | |
| } catch { | |
| Log.general.error("Failed to clear schedule: \(error)") | |
| } | |
| let previousSchedules = scheduleManager.schedules | |
| scheduleManager.schedules = schedules | |
| do { | |
| let api = APIBackend.current.createClient() | |
| scheduleManager.schedules = try await api.updateSchedules(schedules) | |
| Haptics.heavy() | |
| } catch { | |
| scheduleManager.schedules = previousSchedules | |
| Log.general.error("Failed to clear schedule: \(error)") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/ScheduleScreen.swift` around lines 113 - 120, The
code optimistically sets scheduleManager.schedules before persisting with
APIBackend.current.createClient() and api.updateSchedules(schedules); if that
call fails you must revert scheduleManager.schedules to the previous value and
surface an error to the user instead of silently logging it. Capture the
previous schedules (e.g., let previous = scheduleManager.schedules) before
mutating, await api.updateSchedules, and on catch restore
scheduleManager.schedules = previous, avoid triggering Haptics.heavy() on
failure, and present a user-visible error (alert/toast) in addition to
Log.general.error("Failed to clear schedule: \(error)").
… power Previously updateSchedules deleted ALL schedule entries then recreated everything, causing edits to one day to wipe other days. Now only deletes and recreates entries for the specific day being updated. Also skips power schedules that cross midnight (core#205 workaround). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 181-183: The current check converts only the hour prefix of
daily.power.on/off into Ints (onHour/offHour) and defaults parse failures to 0,
which breaks same-hour ranges like "22:15->22:45" and misclassifies malformed
times; instead parse both hour and minute components from daily.power.on and
daily.power.off into total minutes (e.g., onMinutes = hour*60 + minute) using
safe optional parsing (do not default to 0), then compare onMinutes and
offMinutes to determine the window (handle equal-hours by comparing minutes),
and treat parse failures as invalid by returning false or skipping the range;
update the condition around where onHour/offHour are used to use these
total-minute comparisons and handle overnight ranges (offMinutes <= onMinutes
means spans midnight) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d3f75d3-173c-4b2a-a766-7de5e3526024
📒 Files selected for processing (1)
Sleepypod/Networking/SleepypodCoreClient.swift
Rewrites SleepStagesTimelineView as a real hypnogram showing Wake/REM/Light/Deep stages from SleepAnalyzer epochs. Falls back to bed presence chart when no vitals data. Adds tap interaction for HR/HRV/BR tooltip and stage duration legend. Also adds DEVELOPMENT_TEAM for device signing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sitioned - Merge sleep analysis + hypnogram into single card with score ring, distribution bar, and interactive chart - Add compact side toggle pill (shows name + L/R badge) in Health header - Calibration warning checks only selected side, moves into scroll - Reposition link button below side selector on Temp screen - Remove debug logging from fetchVitals Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…arsing - clearSchedule: rollback optimistic update on API failure - loadFromSchedule: reset to defaults before loading, handles nil schedule - SmartCurveView: reload when schedules finish loading (onChange) - Power schedule: parse full HH:mm to minutes for cross-midnight check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
updateSchedulesnow only deletes/recreates entries for days with data instead of wiping all days. Skips cross-midnight power schedules (core#205 workaround)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Style