feat: smart sleep curve — auto temperature schedule#12
Merged
Conversation
Schedule page: - Smart curve generator: set bedtime + wake time → auto temperature profile - Science-backed phases: wind down (+warm), fall asleep (cool), deep sleep (coldest), maintain (cool), pre-wake (warm), wake (neutral) - Three intensity presets: Cool / Balanced / Warm - Interactive chart with catmullRom curve, phase background colors, gradient fills, and offset axis (+/- from base) - "Apply to Schedule" saves the curve as temperature set points + power schedule + alarm time - Phase legend with colored dots - Advanced settings collapsed below (existing manual set points) Models: - SleepCurve: generates Points from bedtime/wake/intensity - CoolingIntensity: Cool (-8° deep), Balanced (-6°), Warm (-4°) - toScheduleTemperatures() converts curve to API format References: Heller 2012, Kräuchi 2007, Czeisler 1999, Walker 2017 Closes #11 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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 (2)
📝 WalkthroughWalkthroughAdds a smart sleep-curve feature: new SleepCurve model, RangeSlider UI, SmartCurveView integrated into ScheduleScreen, HealthKit entitlements and Info.plist key, project wiring for new sources and entitlements, and multi-day selection support in ScheduleManager/DaySelectorView. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SmartCurveView
participant SleepCurve
participant ScheduleManager
participant APIBackend
participant HealthKit
User->>SmartCurveView: Set bedtime, wake time, intensity
SmartCurveView->>SleepCurve: generate(bedtime, wakeTime, intensity)
SleepCurve-->>SmartCurveView: [Point] (time, tempOffset, phase)
SmartCurveView->>SmartCurveView: Render chart & legend
User->>SmartCurveView: Tap "Apply to Schedule"
SmartCurveView->>SleepCurve: toScheduleTemperatures([Point])
SleepCurve-->>SmartCurveView: { "HH:mm": temperature }
SmartCurveView->>ScheduleManager: Update schedules & selectedDays
SmartCurveView->>APIBackend: POST updated schedule
APIBackend-->>SmartCurveView: Success
SmartCurveView->>User: Show success + haptic
User->>SmartCurveView: Tap "Import from Health"
SmartCurveView->>HealthKit: Request sleep data / auth
HealthKit-->>SmartCurveView: Bedtime/wakeTime
SmartCurveView->>User: Populate pickers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Sleepypod/Views/Schedule/SmartCurveView.swift (1)
203-218: Unused variablelabelin legend tuple.The
labelfield in the phases tuple is declared but never referenced in theForEachbody.♻️ Suggested fix
private var phaseLegend: some View { - let phases: [(String, Color, String)] = [ - ("Wind Down", Theme.warming, "+warm"), - ("Fall Asleep", Theme.cooling, "cool"), - ("Deep Sleep", Color(hex: "2563eb"), "coldest"), - ("Pre-Wake", Theme.amber, "+warm"), + let phases: [(name: String, color: Color)] = [ + ("Wind Down", Theme.warming), + ("Fall Asleep", Theme.cooling), + ("Deep Sleep", Color(hex: "2563eb")), + ("Pre-Wake", Theme.amber), ] return HStack(spacing: 12) { - ForEach(phases, id: \.0) { name, color, label in + ForEach(phases, id: \.name) { phase in HStack(spacing: 4) { - Circle().fill(color).frame(width: 6, height: 6) - Text(name) + Circle().fill(phase.color).frame(width: 6, height: 6) + Text(phase.name) .font(.system(size: 9)) .foregroundColor(Theme.textSecondary) } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 203 - 218, The tuple element "label" in the phases array is declared but never used; fix by either removing the third element from the tuple type and each tuple (change phases to [(String, Color)] and drop the label values) or keep the tuple values but ignore the unused value in the ForEach closure by destructuring as (name, color, _) instead of (name, color, label); update the phases declaration and the ForEach closure in SmartCurveView (phases and the closure parameter list) accordingly.Sleepypod/Models/SleepCurve.swift (1)
78-90: Consider documenting key collision behavior.When multiple points map to the same
HH:mmkey (e.g., the duplicate points atpreWakeStart), the last value overwrites earlier ones. This is currently undocumented and could lead to subtle data loss.📝 Suggested documentation
/// Convert curve points to schedule set points (HH:mm → tempF pairs) + /// Note: If multiple points share the same HH:mm timestamp, the last one wins. static func toScheduleTemperatures(_ points: [Point], baseTempF: Int = 80) -> [String: Int] {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Models/SleepCurve.swift` around lines 78 - 90, Add explicit documentation to the toScheduleTemperatures(_ points: [Point], baseTempF: Int = 80) function stating that when multiple Point entries yield the same "HH:mm" key (for example duplicate points at preWakeStart), the later entry in the input array will overwrite earlier values (last-wins). Mention that keys are generated via DateFormatter with "HH:mm" and that callers should deduplicate or order points if they need a different merge behavior. This clarifies the current behavior without changing implementation.
🤖 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/Models/SleepCurve.swift`:
- Around line 62-70: There are duplicate Point entries at preWakeStart (one with
phase .maintain and one with .preWake); remove the earlier append that adds
Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .maintain) so
only the pre-wake points are added at preWakeStart/wake in SleepCurve (affecting
the points array and the preWakeStart logic using deepSleepEnd, preWakeStart,
wake, offsets, and Point).
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 285-296: The success animation and haptic run unconditionally
after the do/catch; move the success UI and Haptics.heavy() into the successful
branch so they only run when the API call succeeds. Specifically, after calling
APIBackend.current.createClient() and assigning scheduleManager.schedules = try
await api.updateSchedules(schedules) inside the do block, set isSaving = false,
trigger withAnimation { showSuccess = true }, call Haptics.heavy(), await the
sleep and then hide showSuccess; in the catch branch ensure isSaving = false is
set and do not trigger showSuccess or Haptics (optionally set/trigger an error
state instead). Ensure you update the code paths around
APIBackend.current.createClient(), scheduleManager.schedules, showSuccess,
isSaving, and Haptics.heavy() accordingly.
---
Nitpick comments:
In `@Sleepypod/Models/SleepCurve.swift`:
- Around line 78-90: Add explicit documentation to the toScheduleTemperatures(_
points: [Point], baseTempF: Int = 80) function stating that when multiple Point
entries yield the same "HH:mm" key (for example duplicate points at
preWakeStart), the later entry in the input array will overwrite earlier values
(last-wins). Mention that keys are generated via DateFormatter with "HH:mm" and
that callers should deduplicate or order points if they need a different merge
behavior. This clarifies the current behavior without changing implementation.
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 203-218: The tuple element "label" in the phases array is declared
but never used; fix by either removing the third element from the tuple type and
each tuple (change phases to [(String, Color)] and drop the label values) or
keep the tuple values but ignore the unused value in the ForEach closure by
destructuring as (name, color, _) instead of (name, color, label); update the
phases declaration and the ForEach closure in SmartCurveView (phases and the
closure parameter list) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 521fa077-8b1a-4cf4-b057-8af656490b5e
📒 Files selected for processing (4)
Sleepypod.xcodeproj/project.pbxprojSleepypod/Models/SleepCurve.swiftSleepypod/Views/Schedule/ScheduleScreen.swiftSleepypod/Views/Schedule/SmartCurveView.swift
Comment on lines
+62
to
+70
| // Maintain: deep sleep end → wake -30min | ||
| let preWakeStart = wake.addingTimeInterval(-30 * 60) | ||
| if preWakeStart > deepSleepEnd { | ||
| points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .maintain)) | ||
| } | ||
|
|
||
| // Pre-wake: wake -30min → wake (warm up) | ||
| points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .preWake)) | ||
| points.append(Point(time: wake, tempOffset: offsets.preWake, phase: .preWake)) |
There was a problem hiding this comment.
Duplicate points at preWakeStart time.
When preWakeStart > deepSleepEnd (the typical case), two points are added at exactly preWakeStart: one with phase .maintain (line 65) and another with phase .preWake (line 69). This creates duplicate entries at the same timestamp, which may cause visual artifacts in the chart and ambiguous phase assignments.
🐛 Proposed fix
// Maintain: deep sleep end → wake -30min
let preWakeStart = wake.addingTimeInterval(-30 * 60)
if preWakeStart > deepSleepEnd {
points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .maintain))
}
// Pre-wake: wake -30min → wake (warm up)
- points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .preWake))
+ // Transition point - offset slightly to avoid duplicate timestamp
+ points.append(Point(time: preWakeStart.addingTimeInterval(1), tempOffset: offsets.maintain, phase: .preWake))
points.append(Point(time: wake, tempOffset: offsets.preWake, phase: .preWake))Alternatively, consider removing the redundant point entirely since the maintain phase already ends at preWakeStart.
📝 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.
Suggested change
| // Maintain: deep sleep end → wake -30min | |
| let preWakeStart = wake.addingTimeInterval(-30 * 60) | |
| if preWakeStart > deepSleepEnd { | |
| points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .maintain)) | |
| } | |
| // Pre-wake: wake -30min → wake (warm up) | |
| points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .preWake)) | |
| points.append(Point(time: wake, tempOffset: offsets.preWake, phase: .preWake)) | |
| // Maintain: deep sleep end → wake -30min | |
| let preWakeStart = wake.addingTimeInterval(-30 * 60) | |
| if preWakeStart > deepSleepEnd { | |
| points.append(Point(time: preWakeStart, tempOffset: offsets.maintain, phase: .maintain)) | |
| } | |
| // Pre-wake: wake -30min → wake (warm up) | |
| // Transition point - offset slightly to avoid duplicate timestamp | |
| points.append(Point(time: preWakeStart.addingTimeInterval(1), tempOffset: offsets.maintain, phase: .preWake)) | |
| points.append(Point(time: wake, tempOffset: offsets.preWake, phase: .preWake)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Models/SleepCurve.swift` around lines 62 - 70, There are duplicate
Point entries at preWakeStart (one with phase .maintain and one with .preWake);
remove the earlier append that adds Point(time: preWakeStart, tempOffset:
offsets.maintain, phase: .maintain) so only the pre-wake points are added at
preWakeStart/wake in SleepCurve (affecting the points array and the preWakeStart
logic using deepSleepEnd, preWakeStart, wake, offsets, and Point).
Comment on lines
+285
to
+296
| do { | ||
| let api = APIBackend.current.createClient() | ||
| scheduleManager.schedules = try await api.updateSchedules(schedules) | ||
| } catch { | ||
| Log.general.error("Failed to save smart curve: \(error)") | ||
| } | ||
|
|
||
| isSaving = false | ||
| withAnimation { showSuccess = true } | ||
| Haptics.heavy() | ||
| try? await Task.sleep(for: .seconds(2)) | ||
| withAnimation { showSuccess = false } |
There was a problem hiding this comment.
Success feedback shown even when save fails.
The success animation and haptic feedback execute after the catch block, meaning users see "Applied!" even when the API call fails. The error is only logged, leaving users unaware of the failure.
🐛 Proposed fix
scheduleManager.schedules = schedules
+ var saveSucceeded = false
do {
let api = APIBackend.current.createClient()
scheduleManager.schedules = try await api.updateSchedules(schedules)
+ saveSucceeded = true
} catch {
Log.general.error("Failed to save smart curve: \(error)")
}
isSaving = false
- withAnimation { showSuccess = true }
- Haptics.heavy()
- try? await Task.sleep(for: .seconds(2))
- withAnimation { showSuccess = false }
+ if saveSucceeded {
+ withAnimation { showSuccess = true }
+ Haptics.heavy()
+ try? await Task.sleep(for: .seconds(2))
+ withAnimation { showSuccess = false }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 285 - 296, The
success animation and haptic run unconditionally after the do/catch; move the
success UI and Haptics.heavy() into the successful branch so they only run when
the API call succeeds. Specifically, after calling
APIBackend.current.createClient() and assigning scheduleManager.schedules = try
await api.updateSchedules(schedules) inside the do block, set isSaving = false,
trigger withAnimation { showSuccess = true }, call Haptics.heavy(), await the
sleep and then hide showSuccess; in the catch branch ensure isSaving = false is
set and do not trigger showSuccess or Haptics (optionally set/trigger an error
state instead). Ensure you update the code paths around
APIBackend.current.createClient(), scheduleManager.schedules, showSuccess,
isSaving, and Haptics.heavy() accordingly.
HealthKit (#13): - "Import from Health" button reads recent sleep sessions - Extracts bedtime + wake time from HKCategoryValueSleepAnalysis - Shows "Synced from Health ✓" on success - Handles permission denied, no data, device not available - Uses inBed or asleep samples from last 7 days Chart: - Y-axis shows user's preferred format (°F, °C, or +/- relative) - Uses TemperatureConversion.displayTemp for consistency Schedule: - Removed redundant controls from Advanced (profile picker, sleep times) - Advanced now just shows manual phase blocks - Schedule toggle moved above Advanced Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
HealthKit: - Added entitlement (com.apple.developer.healthkit) to project.yml - Added NSHealthShareUsageDescription to Info.plist settings - Button renamed "Sync from Apple Health" with red heart icon - Moved to right side of intensity description row (not standalone) - Shows "Synced" green check after successful import Curve smoothing: - ~18 control points (was ~8) for much smoother catmullRom - Gradual transitions: fall asleep ramps over 45min not 30 - Deep sleep → maintain has 30min transition (not instant) - Pre-wake warms gradually over 25min with midpoint - Wake returns to neutral over 20min Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…less Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…elect, smoother curve HealthKit: - Now queries future-dated samples first (iOS Sleep Schedule writes forward-looking inBed samples for tonight/tomorrow) - Falls back to past 7 days of sleep analysis data - Fixed: was only looking at past data, missing the schedule Curve smoothing: - All transitions model real water thermal lag (~15-20 min per °F) - Fall asleep: 90 min gradual ramp (was 30 min) - Deep→maintain: 45 min transition (was instant) - Pre-wake: 45 min warm (was 25 min) - Wake→neutral: 30 min (was 20 min) - ~25 control points for very smooth catmullRom Day selector: - Multi-select: tap multiple days, apply curve to all at once - Must keep at least one day selected - Schedule manager tracks selectedDays set Schedule cleanup: - Advanced only shows manual phase blocks - Schedule toggle above Advanced Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sleepypod/Views/Schedule/ScheduleScreen.swift (1)
42-55:⚠️ Potential issue | 🟡 MinorPhase blocks are always visible despite "Manual Set Points" toggle.
The
showAdvancedstate controls the chevron rotation but doesn't actually show/hide the phase blocks. The button suggests these are "advanced" settings that should be collapsible, but theForEachis rendered unconditionally.🐛 Proposed fix to conditionally show phase blocks
- // Phase blocks - if scheduleManager.schedules != nil { + // Phase blocks (advanced) + if showAdvanced, scheduleManager.schedules != nil { VStack(spacing: 12) { ForEach(scheduleManager.phases) { phase in PhaseBlockView(phase: phase) } }🤖 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 42 - 55, The phase block list is always rendered even when the "Manual Set Points" toggle is collapsed; update the view so the VStack/ForEach that builds PhaseBlockView (ForEach(scheduleManager.phases) { phase in PhaseBlockView(phase: phase) }) is only shown when the local state showAdvanced is true (in addition to the existing scheduleManager.schedules/isLoading checks). Concretely, wrap the existing VStack( spacing: 12 ) { ForEach(...) } branch in a conditional that checks showAdvanced (preserving the current else-if isLoading and fallback Text branches), and keep the chevron rotation logic tied to showAdvanced so the header toggle controls the visibility of the PhaseBlockView list.
♻️ Duplicate comments (1)
Sleepypod/Views/Schedule/SmartCurveView.swift (1)
406-417:⚠️ Potential issue | 🟠 MajorSuccess feedback still shown even when save fails.
The success animation and haptic feedback execute unconditionally after the
catchblock, meaning users see "Applied!" even when the API call fails.🐛 Proposed fix
scheduleManager.schedules = schedules + var saveSucceeded = false do { let api = APIBackend.current.createClient() scheduleManager.schedules = try await api.updateSchedules(schedules) + saveSucceeded = true } catch { Log.general.error("Failed to save smart curve: \(error)") } isSaving = false - withAnimation { showSuccess = true } - Haptics.heavy() - try? await Task.sleep(for: .seconds(2)) - withAnimation { showSuccess = false } + if saveSucceeded { + withAnimation { showSuccess = true } + Haptics.heavy() + try? await Task.sleep(for: .seconds(2)) + withAnimation { showSuccess = false } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 406 - 417, The success animation and haptic are being run regardless of failure; update the control flow so the success UI/haptic (setting showSuccess = true withAnimation and Haptics.heavy(), the 2s sleep, and hiding showSuccess) only runs after a successful await of APIBackend.current.createClient().updateSchedules(...) inside the do block (where you set scheduleManager.schedules), and ensure isSaving is still cleared in both success and catch paths (set isSaving = false in a finally-equivalent location or set it in both the do and catch); reference the symbols APIBackend.current.createClient(), updateSchedules, scheduleManager.schedules, isSaving, showSuccess, and Haptics.heavy() to locate and change the code accordingly.
🧹 Nitpick comments (2)
Sleepypod/Views/Schedule/SmartCurveView.swift (2)
238-255: Unusedlabelvariable in phase legend tuple.The
labelcomponent of the tuple (e.g., "+warm", "cool") is destructured but never used in the view.♻️ Proposed cleanup
private var phaseLegend: some View { - let phases: [(String, Color, String)] = [ - ("Wind Down", Theme.warming, "+warm"), - ("Fall Asleep", Theme.cooling, "cool"), - ("Deep Sleep", Color(hex: "2563eb"), "coldest"), - ("Pre-Wake", Theme.amber, "+warm"), + let phases: [(String, Color)] = [ + ("Wind Down", Theme.warming), + ("Fall Asleep", Theme.cooling), + ("Deep Sleep", Color(hex: "2563eb")), + ("Pre-Wake", Theme.amber), ] return HStack(spacing: 12) { - ForEach(phases, id: \.0) { name, color, label in + ForEach(phases, id: \.0) { name, color in HStack(spacing: 4) { Circle().fill(color).frame(width: 6, height: 6) Text(name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 238 - 255, The tuple element "label" in the phases array is destructured but never used; in the phaseLegend computed var remove the third tuple element from phases (use [(String, Color)] instead of [(String, Color, String)]) and update the ForEach closure to only unpack (name, color), or if the label is intended to be shown, use the destructured label inside the HStack (e.g., add a Text(label) with appropriate styling). Update the phases declaration and the ForEach signature (and any references to label) in phaseLegend to keep the code consistent.
291-297: Potential data race with strict concurrency enabled.With
SWIFT_STRICT_CONCURRENCY: completeenabled, updating@Stateproperties from a non-@MainActorcontext can trigger data race warnings or runtime issues. TherequestAuthorizationcallback executes on a background queue, and while you dispatch to main for state updates, the closure itself capturesselfin a non-isolated context.Consider using the async/await HealthKit API (
requestAuthorization(toShare:read:)async variant) within aTask {@mainactorin ... }block, or mark the entire function with@MainActor.♻️ Suggested approach using async API
`@MainActor` private func importFromHealth() async { guard HKHealthStore.isHealthDataAvailable() else { healthError = "Health data not available on this device" return } let store = HKHealthStore() let sleepType = HKCategoryType(.sleepAnalysis) do { try await store.requestAuthorization(toShare: [], read: [sleepType]) // ... rest of query logic using async APIs } catch { healthError = "Health access denied — enable in Settings → Privacy → Health" } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 291 - 297, The requestAuthorization callback is capturing self on a background thread and then mutating the `@State` property healthError, which can cause strict-concurrency data races; to fix, convert the import flow to use the async HealthKit API or ensure main-actor isolation: mark the surrounding method (e.g., importFromHealth) as `@MainActor` or wrap async work with Task { `@MainActor` in ... }, call the async requestAuthorization(toShare:read:) variant with try/await (or call DispatchQueue.main only inside a `@MainActor` context), and then assign healthError directly on the main actor so the closure does not capture non-isolated self.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@project.yml`:
- Around line 28-33: Remove the unnecessary Clinical Health Records entitlement
by deleting the "health-records" entry from the
com.apple.developer.healthkit.access list in the entitlements (the block under
entitlements -> path: Sleepypod/Sleepypod.entitlements); leave
com.apple.developer.healthkit: true intact since SmartCurveView.swift only uses
HKCategoryType(.sleepAnalysis) and does not require HKClinicalRecord APIs, so do
not add any other clinical entitlements unless you later use HKClinicalRecord or
related Clinical Health Record APIs.
In `@Sleepypod/Models/SleepCurve.swift`:
- Around line 59-76: The phase timeline breaks for very short sleepDuration
values causing overlap between deepSleepEnd/transitionEnd and preWakeStart; fix
by enforcing a minimum effective duration or clamping phase boundaries before
appending points: compute an effectiveSleep = max(sleepDuration,
minDurationInSeconds) or clamp deepSleepEnd/transitionEnd so transitionEnd <=
preWakeStart, and only append Point(...) for deepSleepEnd, transitionEnd,
maintainMid, and preWakeStart when their times are strictly increasing (or
adjust their times proportionally); update the code paths referencing
deepSleepEnd, transitionEnd, preWakeStart, and points to use the
clamped/effective times to ensure phases remain chronological and semantically
correct.
---
Outside diff comments:
In `@Sleepypod/Views/Schedule/ScheduleScreen.swift`:
- Around line 42-55: The phase block list is always rendered even when the
"Manual Set Points" toggle is collapsed; update the view so the VStack/ForEach
that builds PhaseBlockView (ForEach(scheduleManager.phases) { phase in
PhaseBlockView(phase: phase) }) is only shown when the local state showAdvanced
is true (in addition to the existing scheduleManager.schedules/isLoading
checks). Concretely, wrap the existing VStack( spacing: 12 ) { ForEach(...) }
branch in a conditional that checks showAdvanced (preserving the current else-if
isLoading and fallback Text branches), and keep the chevron rotation logic tied
to showAdvanced so the header toggle controls the visibility of the
PhaseBlockView list.
---
Duplicate comments:
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 406-417: The success animation and haptic are being run regardless
of failure; update the control flow so the success UI/haptic (setting
showSuccess = true withAnimation and Haptics.heavy(), the 2s sleep, and hiding
showSuccess) only runs after a successful await of
APIBackend.current.createClient().updateSchedules(...) inside the do block
(where you set scheduleManager.schedules), and ensure isSaving is still cleared
in both success and catch paths (set isSaving = false in a finally-equivalent
location or set it in both the do and catch); reference the symbols
APIBackend.current.createClient(), updateSchedules, scheduleManager.schedules,
isSaving, showSuccess, and Haptics.heavy() to locate and change the code
accordingly.
---
Nitpick comments:
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 238-255: The tuple element "label" in the phases array is
destructured but never used; in the phaseLegend computed var remove the third
tuple element from phases (use [(String, Color)] instead of [(String, Color,
String)]) and update the ForEach closure to only unpack (name, color), or if the
label is intended to be shown, use the destructured label inside the HStack
(e.g., add a Text(label) with appropriate styling). Update the phases
declaration and the ForEach signature (and any references to label) in
phaseLegend to keep the code consistent.
- Around line 291-297: The requestAuthorization callback is capturing self on a
background thread and then mutating the `@State` property healthError, which can
cause strict-concurrency data races; to fix, convert the import flow to use the
async HealthKit API or ensure main-actor isolation: mark the surrounding method
(e.g., importFromHealth) as `@MainActor` or wrap async work with Task { `@MainActor`
in ... }, call the async requestAuthorization(toShare:read:) variant with
try/await (or call DispatchQueue.main only inside a `@MainActor` context), and
then assign healthError directly on the main actor so the closure does not
capture non-isolated self.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92e0c7a0-f435-40ca-92a1-b2ad769e46cc
📒 Files selected for processing (6)
Sleepypod.xcodeproj/project.pbxprojSleepypod/Models/SleepCurve.swiftSleepypod/Sleepypod.entitlementsSleepypod/Views/Schedule/ScheduleScreen.swiftSleepypod/Views/Schedule/SmartCurveView.swiftproject.yml
✅ Files skipped from review due to trivial changes (1)
- Sleepypod/Sleepypod.entitlements
Comment on lines
+28
to
+33
| entitlements: | ||
| path: Sleepypod/Sleepypod.entitlements | ||
| properties: | ||
| com.apple.developer.healthkit: true | ||
| com.apple.developer.healthkit.access: | ||
| - health-records |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
iOS HealthKit health-records vs standard sleep analysis entitlements
💡 Result:
Standard HealthKit sleep analysis (e.g., HKCategoryTypeIdentifier.sleepAnalysis)
- Entitlement / capability: Just enable the HealthKit capability, which adds the
com.apple.developer.healthkit.accessentitlement to the app. (Some related HealthKit entitlements, like background delivery, are separate toggles, but sleep itself doesn’t require a special entitlement beyond HealthKit.) [1] - Info.plist:
NSHealthShareUsageDescription(read) andNSHealthUpdateUsageDescription(write, only if you write) [2]. - Runtime permission: Use
HKHealthStore.requestAuthorization(...)for.sleepAnalysislike any other HealthKit type [2].
HealthKit “Health Records” / Clinical Records (e.g., HKClinicalRecord, HKClinicalTypeIdentifier.*)
- Extra capability beyond basic HealthKit: You must enable the Health Records / Clinical Health Records capability for the app (App Store Connect / developer portal); Apple will reject apps that reference the Clinical Health Records API without an appropriate primary feature/use case [3].
- Info.plist: Add
NSHealthClinicalHealthRecordsShareUsageDescription(purpose string specifically for clinical records) [4]. - Authorization characteristics: Clinical record samples are read-only (you can’t request “share/write” authorization for clinical record types) [5], and you query/read them via
HKClinicalRecord[6]. - Optional “required types” declaration: If your app requires certain clinical record types to function, list them under
NSHealthRequiredReadAuthorizationTypeIdentifiers[2].
Verifiable Health Records (SMART Health Cards, etc.)
- Not the same as general Health Records: Accessing verifiable health records uses a dedicated flow and (per Apple’s WWDC guidance) requires requesting a specific HealthKit access entitlement for Verifiable Health Records [7].
Sources:
[1] Apple Developer Documentation — com.apple.developer.healthkit.background-delivery (notes key com.apple.developer.healthkit.access) https://developer.apple.com/documentation/bundleresources/entitlements/com.apple.developer.healthkit.background-delivery
[2] Apple Developer Documentation — Authorizing access to health data https://developer.apple.com/documentation/healthkit/authorizing-access-to-health-data
[3] Stack Overflow (quoting App Review guidance) — Health Records capability / Clinical Health Records API rejection context https://stackoverflow.com/questions/63009308/cant-seem-to-disable-the-health-records-capability-within-healthkit-in-either-x
[4] Apple Info.plist Key Reference — NSHealthClinicalHealthRecordsShareUsageDescription https://developer-mdn.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CocoaKeys.html
[5] Apple Developer Documentation — HKClinicalTypeIdentifier (clinical records are read-only) https://developer.apple.com/documentation/healthkit/hkclinicaltypeidentifier
[6] Apple Developer Documentation — HKClinicalRecord https://developer.apple.com/documentation/healthkit/hkclinicalrecord
[7] Apple Developer (WWDC21) — Explore Verifiable Health Records (mentions special entitlement) https://developer.apple.com/videos/play/wwdc2021/10089/
🏁 Script executed:
# Find and examine the Sleepypod.entitlements file
fd -t f "Sleepypod.entitlements" -x cat {}Repository: sleepypod/ios
Length of output: 397
🏁 Script executed:
# Find SmartCurveView.swift and check HealthKit usage
fd -t f "SmartCurveView.swift" -x cat {}Repository: sleepypod/ios
Length of output: 16275
🏁 Script executed:
# Search for any clinical health record references in the codebase
rg "HKClinical|NSHealthClinicalHealthRecordsShareUsageDescription" --type swift --type plistRepository: sleepypod/ios
Length of output: 86
🏁 Script executed:
# Check project.yml for the full entitlements context
fd -t f "project.yml" -x head -50 {}Repository: sleepypod/ios
Length of output: 1665
🌐 Web query:
com.apple.developer.healthkit.access entitlement array valid values iOS
💡 Result:
For iOS, the entitlement com.apple.developer.healthkit.access is an array of strings used to declare HealthKit data types that require additional permission beyond the base com.apple.developer.healthkit entitlement. [1]
Valid array values
health-records— enables Clinical Health Records access (the “Health Records”/clinical records capability). This is the value that appears in provisioning profiles and entitlements when Clinical Health Records is enabled. [2][3]
Apple’s public docs/tools surface health-records as the (currently) known/used value for this entitlement. [1][2][3]
Sources:
[1] Apple Developer Documentation (Entitlements reference / HealthKit entitlements index via background-delivery page) (developer.apple.com)
[2] Stack Overflow (provisioning profile shows com.apple.developer.healthkit.access = health-records) (stackoverflow.com)
[3] NativeScript plugin docs showing the entitlement value health-records (market.nativescript.org)
Citations:
- 1: https://developer.apple.com/documentation/bundleresources/entitlements/com.apple.developer.healthkit.background-delivery?utm_source=openai
- 2: https://stackoverflow.com/questions/63009308/cant-seem-to-disable-the-health-records-capability-within-healthkit-in-either-x?utm_source=openai
- 3: https://market.nativescript.org/plugins/%40webileapps/nativescript-healthkit/?utm_source=openai
🌐 Web query:
Apple HealthKit entitlement access property list values documentation
💡 Result:
For HealthKit, the entitlement values you put in your app’s .entitlements (plist) are:
com.apple.developer.healthkit: Boolean (true) — enables the app to request HealthKit authorization. (developer.apple.com)com.apple.developer.healthkit.access: Array of strings — used only for HealthKit features that require additional permission beyond basic HealthKit. (developer.apple.com)- A commonly-used (and App Review–relevant) value is
health-records(Clinical Health Records). (stackoverflow.com)
- A commonly-used (and App Review–relevant) value is
- (Related)
com.apple.developer.healthkit.background-delivery: Boolean (true) — allows background delivery forHKObserverQuery. (developer.apple.com)
Minimal example:
<!-- YourApp.entitlements -->
<plist version="1.0">
<dict>
<key>com.apple.developer.healthkit</key>
<true/>
<!-- Only if you actually use Clinical Health Records APIs/features -->
<key>com.apple.developer.healthkit.access</key>
<array>
<string>health-records</string>
</array>
</dict>
</plist>Also ensure your Info.plist includes the required privacy strings, e.g.:
NSHealthShareUsageDescriptionNSHealthUpdateUsageDescription(developer.apple.com)
Citations:
- 1: https://developer.apple.com/documentation/bundleresources/entitlements/com.apple.developer.healthkit.background-delivery?utm_source=openai
- 2: https://developer.apple.com/documentation/bundleresources/entitlements/com.apple.developer.healthkit.background-delivery?utm_source=openai
- 3: https://stackoverflow.com/questions/63009308/cant-seem-to-disable-the-health-records-capability-within-healthkit-in-either-x?utm_source=openai
- 4: https://developer.apple.com/documentation/bundleresources/entitlements/com.apple.developer.healthkit.background-delivery?utm_source=openai
- 5: https://developer.apple.com/documentation/BundleResources/Information-Property-List/NSHealthShareUsageDescription?utm_source=openai
Remove unnecessary health-records entitlement; it's not used by this app.
The entitlements declare com.apple.developer.healthkit.access: [health-records], which enables Clinical Health Records access. However, SmartCurveView.swift uses only HKCategoryType(.sleepAnalysis), which is standard HealthKit requiring no special entitlements beyond com.apple.developer.healthkit: true.
The health-records value should be removed unless you later add Clinical Health Record APIs (e.g., HKClinicalRecord). Declaring this capability without a justified use case may cause App Review rejection or scrutiny.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@project.yml` around lines 28 - 33, Remove the unnecessary Clinical Health
Records entitlement by deleting the "health-records" entry from the
com.apple.developer.healthkit.access list in the entitlements (the block under
entitlements -> path: Sleepypod/Sleepypod.entitlements); leave
com.apple.developer.healthkit: true intact since SmartCurveView.swift only uses
HKCategoryType(.sleepAnalysis) and does not require HKClinicalRecord APIs, so do
not add any other clinical entitlements unless you later use HKClinicalRecord or
related Clinical Health Record APIs.
Temp range: - Two sliders: Coolest (55-78°F) and Warmest (78-110°F) - Curve offsets scale proportionally to the user's range - Chart Y-axis adapts to the selected range - Shows "68° – 86°F" summary Health sync: - Removed "Sync from Apple Health" button — Apple does not expose the Sleep Schedule through any public API (confirmed via Apple Developer Forums, HealthKit docs, WWDC 2025). sleepAnalysis only contains historical data, not the configured schedule. - Filed as known limitation in iOS #13 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Range slider: - Single track, two draggable thumbs (blue=cool, orange=warm) - Gradient fill between thumbs (blue → gray → orange) - Haptic feedback on drag - Thumbs can't cross each other - Labels show both temps in user's preferred format Chart: - Fixed Y-axis (-20 to +20) — doesn't rescale with slider changes - Curve scales to the user's range but chart stays stable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Slider moved below chart so dragging doesn't cover it. Adaptive range: centers on current midpoint ±15°F. So if you're at 80-85°F, the slider shows 67-97°F instead of 55-110°F, giving much more resolution for fine adjustments. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Temp range: - Replaced dual slider with horizontal drag lines on the chart - Blue dashed line = min (drag up/down), orange = max - Labels on right edge show current temp - "Drag lines to adjust" hint below Chart fixes: - Bedtime defaults to tonight 22:00, wake to tomorrow 07:00 (was defaulting to today which missed pre-midnight hours) - Removed phase background rectangles (gaps between stages) - Removed separate RangeSlider component (replaced by chart overlay) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
Sleepypod/Views/Schedule/SmartCurveView.swift (1)
501-513:⚠️ Potential issue | 🟠 MajorShow “Applied!” only when the API save succeeds.
Failure path (Line 505-507) still falls through to success animation/haptic at Line 510-513, so users get false success feedback.
Proposed fix
scheduleManager.schedules = schedules + var saveSucceeded = false do { let api = APIBackend.current.createClient() scheduleManager.schedules = try await api.updateSchedules(schedules) + saveSucceeded = true } catch { Log.general.error("Failed to save smart curve: \(error)") } isSaving = false - withAnimation { showSuccess = true } - Haptics.heavy() - try? await Task.sleep(for: .seconds(2)) - withAnimation { showSuccess = false } + if saveSucceeded { + withAnimation { showSuccess = true } + Haptics.heavy() + try? await Task.sleep(for: .seconds(2)) + withAnimation { showSuccess = false } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 501 - 513, The code currently sets scheduleManager.schedules and then calls the API (APIBackend.current.createClient()/updateSchedules) but always proceeds to clear isSaving, show the success animation/haptic (isSaving = false, withAnimation { showSuccess = true }, Haptics.heavy(), sleep, withAnimation { showSuccess = false }) even when the API throws; change the flow so that scheduleManager.schedules is only updated with the API result (assign the result of try await api.updateSchedules(schedules) to scheduleManager.schedules) and move the success UI/haptic block (isSaving = false, showSuccess animation, Haptics.heavy(), sleep, hide success) into the success path after the await, and ensure the catch block sets isSaving = false and does not trigger showSuccess (or returns/throws) so failure does not show “Applied!”.Sleepypod/Models/SleepCurve.swift (1)
84-119:⚠️ Potential issue | 🟡 MinorVery short sleep windows can still produce overlapped/semantically broken phase timelines.
For short durations,
preWakeStartcan move into earlier phases, and sorting hides timeline validity problems instead of preventing them. Add minimum-duration fallback or clamp boundaries to strictly increasing times before appending points.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Models/SleepCurve.swift` around lines 84 - 119, The phase timeline can produce non-monotonic/overlapping Point.times for short sleeps (variables: coolRamp, deepTransition, deepStart, deepSleepEnd, maintainStart, preWakeStart, points), so enforce strictly increasing timestamps before appending: introduce a small minPhaseGap (e.g. 1–5 minutes) and clamp each computed start time to be at least previousPoint.time.addingTimeInterval(minPhaseGap); adjust maintainStart and preWakeStart using max(desired, previous + minGap) and only append Points when their times are strictly greater than the last appended point.time; remove reliance on later sorting to hide invalid timelines and ensure phases (coolDown → deepSleep → maintain → preWake) always advance monotonically.
🧹 Nitpick comments (1)
Sleepypod/Views/Schedule/SmartCurveView.swift (1)
503-504: Prefer using ScheduleManager’s API boundary instead of creating a client in the view.Calling
APIBackend.current.createClient()directly inSmartCurveViewbypasses your injected manager dependency and makes this flow harder to test consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 503 - 504, The view is directly calling APIBackend.current.createClient() and then api.updateSchedules(schedules), which bypasses the injected ScheduleManager; instead, add or use a ScheduleManager API boundary method (e.g., ScheduleManager.updateSchedules(_:), or a clearly named wrapper like performRemoteUpdate(schedules:)) that encapsulates creating the API client and performing api.updateSchedules(schedules), and replace the view call with scheduleManager.updateSchedules(schedules) so the networking is owned by ScheduleManager and the view remains testable.
🤖 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/Models/SleepCurve.swift`:
- Around line 49-58: The scale behavior can invert when coolRoom or warmRoom is
negative; clamp both available ranges to non-negative values before using them
in scale(_ offset: Int). Replace the computed coolRoom and warmRoom with clamped
versions (e.g., coolRoomClamped = max(baseTempF - minTempF, 0) and
warmRoomClamped = max(maxTempF - baseTempF, 0)) and use those in the scale
function, and if a clamped range is zero ensure scale returns 0 for that
direction to avoid inverted or division-by-zero effects in SleepCurve.scale(_:).
In `@Sleepypod/Services/ScheduleManager.swift`:
- Line 9: selectedDays was added but write APIs still only mutate selectedDay,
causing inconsistent persistence; update togglePowerSchedule, updateAlarmTime,
updateBedtime, updatePhaseTemperature, and applyProfile to apply their changes
for every day in selectedDays (e.g., iterate selectedDays and call the existing
per-day write/persist logic or extract that per-day write into a helper and
invoke it for each DayOfWeek), ensuring UI state updates reflect the full set
and using the same persistence/notification path so multi-day edits are actually
saved for each day rather than only selectedDay.
In `@Sleepypod/Views/Schedule/DaySelectorView.swift`:
- Around line 12-17: The bug is that scheduleManager.selectedDay is set
unconditionally which can leave selectedDay pointing to a deselected day; in the
DaySelectorView handler that currently mutates scheduleManager.selectedDays and
then sets scheduleManager.selectedDay, change the logic so selectedDay is only
set when a day is being selected, and when a day is being removed (in the branch
that calls scheduleManager.selectedDays.remove(day)) update
scheduleManager.selectedDay to a valid value (e.g., set it to nil or to a
remaining selectedDays.first) to keep scheduleManager.selectedDay synchronized
with scheduleManager.selectedDays.
In `@Sleepypod/Views/Schedule/RangeSlider.swift`:
- Around line 12-17: The drag math for the thumbs uses local coordinates and
unguarded divisions which can produce jumps and NaN/Inf; update the track view
produced inside GeometryReader to set a named coordinate space (e.g.,
.coordinateSpace(name: "track")), switch the thumbs' DragGesture to
DragGesture(coordinateSpace: .named("track")) so value.location.x is
track-relative, and add defensive guards around all divisors used in the mapping
logic (check step != 0, span = range.upperBound - range.lowerBound != 0, and
width = geo.size.width - thumbSize > 0) inside whatever functions/closures
compute the normalized position and step-snapped value (refer to span, range,
thumbSize, and the DragGesture handlers); if any divisor is invalid,
early-return or clamp to a well-defined fallback (current bound or min/max) to
avoid NaN/Inf and discontinuous jumps.
---
Duplicate comments:
In `@Sleepypod/Models/SleepCurve.swift`:
- Around line 84-119: The phase timeline can produce non-monotonic/overlapping
Point.times for short sleeps (variables: coolRamp, deepTransition, deepStart,
deepSleepEnd, maintainStart, preWakeStart, points), so enforce strictly
increasing timestamps before appending: introduce a small minPhaseGap (e.g. 1–5
minutes) and clamp each computed start time to be at least
previousPoint.time.addingTimeInterval(minPhaseGap); adjust maintainStart and
preWakeStart using max(desired, previous + minGap) and only append Points when
their times are strictly greater than the last appended point.time; remove
reliance on later sorting to hide invalid timelines and ensure phases (coolDown
→ deepSleep → maintain → preWake) always advance monotonically.
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 501-513: The code currently sets scheduleManager.schedules and
then calls the API (APIBackend.current.createClient()/updateSchedules) but
always proceeds to clear isSaving, show the success animation/haptic (isSaving =
false, withAnimation { showSuccess = true }, Haptics.heavy(), sleep,
withAnimation { showSuccess = false }) even when the API throws; change the flow
so that scheduleManager.schedules is only updated with the API result (assign
the result of try await api.updateSchedules(schedules) to
scheduleManager.schedules) and move the success UI/haptic block (isSaving =
false, showSuccess animation, Haptics.heavy(), sleep, hide success) into the
success path after the await, and ensure the catch block sets isSaving = false
and does not trigger showSuccess (or returns/throws) so failure does not show
“Applied!”.
---
Nitpick comments:
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 503-504: The view is directly calling
APIBackend.current.createClient() and then api.updateSchedules(schedules), which
bypasses the injected ScheduleManager; instead, add or use a ScheduleManager API
boundary method (e.g., ScheduleManager.updateSchedules(_:), or a clearly named
wrapper like performRemoteUpdate(schedules:)) that encapsulates creating the API
client and performing api.updateSchedules(schedules), and replace the view call
with scheduleManager.updateSchedules(schedules) so the networking is owned by
ScheduleManager and the view remains testable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9bb83235-4739-41ee-a596-2575e1fdf784
📒 Files selected for processing (6)
Sleepypod.xcodeproj/project.pbxprojSleepypod/Models/SleepCurve.swiftSleepypod/Services/ScheduleManager.swiftSleepypod/Views/Schedule/DaySelectorView.swiftSleepypod/Views/Schedule/RangeSlider.swiftSleepypod/Views/Schedule/SmartCurveView.swift
Comment on lines
+49
to
+58
| let coolRoom = baseTempF - minTempF // how much cooling is available | ||
| let warmRoom = maxTempF - baseTempF // how much warming is available | ||
|
|
||
| func scale(_ offset: Int) -> Int { | ||
| if offset < 0 { | ||
| // Cooling: scale to available cool range | ||
| return -min(abs(offset) * coolRoom / 10, coolRoom) | ||
| } else if offset > 0 { | ||
| // Warming: scale to available warm range | ||
| return min(offset * warmRoom / 8, warmRoom) |
There was a problem hiding this comment.
Clamp warm/cool capacity to non-negative values before scaling.
With current math, minTempF > 80 or maxTempF < 80 makes coolRoom/warmRoom negative, which can invert warming/cooling offsets (wrong curve direction).
Proposed fix
- let baseTempF = 80
- let coolRoom = baseTempF - minTempF // how much cooling is available
- let warmRoom = maxTempF - baseTempF // how much warming is available
+ let baseTempF = 80
+ let normalizedMin = min(minTempF, maxTempF)
+ let normalizedMax = max(minTempF, maxTempF)
+ let coolRoom = max(0, baseTempF - normalizedMin) // how much cooling is available
+ let warmRoom = max(0, normalizedMax - baseTempF) // how much warming is available🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Models/SleepCurve.swift` around lines 49 - 58, The scale behavior
can invert when coolRoom or warmRoom is negative; clamp both available ranges to
non-negative values before using them in scale(_ offset: Int). Replace the
computed coolRoom and warmRoom with clamped versions (e.g., coolRoomClamped =
max(baseTempF - minTempF, 0) and warmRoomClamped = max(maxTempF - baseTempF, 0))
and use those in the scale function, and if a clamped range is zero ensure scale
returns 0 for that direction to avoid inverted or division-by-zero effects in
SleepCurve.scale(_:).
| final class ScheduleManager { | ||
| var schedules: Schedules? | ||
| var selectedDay: DayOfWeek = .monday | ||
| var selectedDays: Set<DayOfWeek> = [.monday] |
There was a problem hiding this comment.
selectedDays is introduced, but ScheduleManager write APIs still mutate only one day.
Line 9 adds multi-day state, but togglePowerSchedule, updateAlarmTime, updateBedtime, updatePhaseTemperature, and applyProfile still only update selectedDay. This creates inconsistent behavior versus bulk paths (for example, SmartCurve apply), and users can think multiple days are edited when only one is persisted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Services/ScheduleManager.swift` at line 9, selectedDays was added
but write APIs still only mutate selectedDay, causing inconsistent persistence;
update togglePowerSchedule, updateAlarmTime, updateBedtime,
updatePhaseTemperature, and applyProfile to apply their changes for every day in
selectedDays (e.g., iterate selectedDays and call the existing per-day
write/persist logic or extract that per-day write into a helper and invoke it
for each DayOfWeek), ensuring UI state updates reflect the full set and using
the same persistence/notification path so multi-day edits are actually saved for
each day rather than only selectedDay.
Comment on lines
+12
to
17
| if isSelected && scheduleManager.selectedDays.count > 1 { | ||
| scheduleManager.selectedDays.remove(day) | ||
| } else if !isSelected { | ||
| scheduleManager.selectedDays.insert(day) | ||
| } | ||
| scheduleManager.selectedDay = day |
There was a problem hiding this comment.
Keep selectedDay synchronized with selectedDays after deselection.
Line 17 runs unconditionally. When a day is removed at Line 13, selectedDay can remain set to a non-selected day.
Proposed fix
- if isSelected && scheduleManager.selectedDays.count > 1 {
- scheduleManager.selectedDays.remove(day)
- } else if !isSelected {
- scheduleManager.selectedDays.insert(day)
- }
- scheduleManager.selectedDay = day
+ if isSelected && scheduleManager.selectedDays.count > 1 {
+ scheduleManager.selectedDays.remove(day)
+ if scheduleManager.selectedDay == day {
+ scheduleManager.selectedDay =
+ DayOfWeek.weekdays.first(where: { scheduleManager.selectedDays.contains($0) }) ?? day
+ }
+ } else if !isSelected {
+ scheduleManager.selectedDays.insert(day)
+ scheduleManager.selectedDay = day
+ } else {
+ scheduleManager.selectedDay = day
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/DaySelectorView.swift` around lines 12 - 17, The bug
is that scheduleManager.selectedDay is set unconditionally which can leave
selectedDay pointing to a deselected day; in the DaySelectorView handler that
currently mutates scheduleManager.selectedDays and then sets
scheduleManager.selectedDay, change the logic so selectedDay is only set when a
day is being selected, and when a day is being removed (in the branch that calls
scheduleManager.selectedDays.remove(day)) update scheduleManager.selectedDay to
a valid value (e.g., set it to nil or to a remaining selectedDays.first) to keep
scheduleManager.selectedDay synchronized with scheduleManager.selectedDays.
Comment on lines
+12
to
+17
| private var span: Double { range.upperBound - range.lowerBound } | ||
|
|
||
| var body: some View { | ||
| GeometryReader { geo in | ||
| let width = geo.size.width - thumbSize | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In SwiftUI DragGesture, is DragGesture.Value.location reported in the local coordinate space of the view that owns the gesture, and what is the recommended way to map thumb drag to a parent track value?
💡 Result:
Yes. DragGesture reports value.location (and startLocation) in the gesture’s coordinateSpace, which defaults to .local—i.e., the local coordinate space of the view the gesture is attached to. The initializer’s default is coordinateSpace: .local. [1] And “local” means the coordinates are relative to that gesture-owning view (vs .global). [2]
Recommended pattern for a thumb-on-track (slider) mapping
- Put a named coordinate space on the track (the parent you want coordinates relative to), e.g.
.coordinateSpace(name: "track"). - Build the thumb’s drag as
DragGesture(..., coordinateSpace: .named("track")), sovalue.location.xis in track coordinates, even though the gesture is attached to the thumb. [1] - Convert position → value using the track width from
GeometryReader, then clamp:
struct MySlider: View {
`@Binding` var value: Double // in 0...1
var body: some View {
GeometryReader { geo in
let w = geo.size.width
let x = CGFloat(value) * w
ZStack(alignment: .leading) {
Capsule().frame(height: 4)
Circle()
.frame(width: 28, height: 28)
.offset(x: x - 14) // center on x
.gesture(
DragGesture(minimumDistance: 0, coordinateSpace: .named("track"))
.onChanged { g in
let t = min(max(0, g.location.x), w) / w
value = Double(t)
}
)
}
.coordinateSpace(name: "track")
}
.frame(height: 28)
}
}This avoids “thumb-local” coordinates and makes the math stable even if the thumb moves. [1]
Sources: [1] [2]
🏁 Script executed:
fd -t f RangeSlider.swiftRepository: sleepypod/ios
Length of output: 99
🏁 Script executed:
cat -n Sleepypod/Views/Schedule/RangeSlider.swiftRepository: sleepypod/ios
Length of output: 3563
Range slider drag math has coordinate space and divide-by-zero hazards.
The DragGesture on the thumbs uses the default .local coordinate space (attached to offset thumb views), but the math treats value.location.x as track-relative. This can cause discontinuous jumps and incorrect mappings, especially during drag. Additionally, divisions by step, span, and width lack guards—if step is set to 0, span is 0 (point range), or layout yields width ≤ 0, the math produces invalid states (NaN/Inf).
Apply a named coordinate space (e.g., .coordinateSpace(name: "track") on the track, DragGesture(coordinateSpace: .named("track")) on thumbs) and add guards for zero-valued divisors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/RangeSlider.swift` around lines 12 - 17, The drag
math for the thumbs uses local coordinates and unguarded divisions which can
produce jumps and NaN/Inf; update the track view produced inside GeometryReader
to set a named coordinate space (e.g., .coordinateSpace(name: "track")), switch
the thumbs' DragGesture to DragGesture(coordinateSpace: .named("track")) so
value.location.x is track-relative, and add defensive guards around all divisors
used in the mapping logic (check step != 0, span = range.upperBound -
range.lowerBound != 0, and width = geo.size.width - thumbSize > 0) inside
whatever functions/closures compute the normalized position and step-snapped
value (refer to span, range, thumbSize, and the DragGesture handlers); if any
divisor is invalid, early-return or clamp to a well-defined fallback (current
bound or min/max) to avoid NaN/Inf and discontinuous jumps.
- Lines inset 30pt from left to avoid Y-axis legend overlap - Wide tap target (±15pt) for easier dragging - Lines clamp to chart bounds - X-axis uses .hour() format — shows "10 PM", "2 AM" etc. - Removed haptics from drag onChange (too frequent, causes lag) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Lines now use chartOverlay + proxy.position(forY:) to map offset to exact chart pixel coordinates. No more manual GeometryReader math that didn't account for axis label padding. - Drag uses proxy.value(atY:) to convert touch position back to offset - Lines start at the plot area edge (after Y-axis), end at right edge - Min line bisects the chart at exactly the min temp offset - X-axis uses 2-hour stride to show all hours including pre-midnight (10 PM, 12 AM, 2 AM, 4 AM, 6 AM, 8 AM) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Drag lines: - 1pt solid at 40% opacity (was 2pt at 60%) — chart visible underneath - Labels removed from lines — temp shown as chips above chart Temp chips: - Between time pickers and intensity: ❄️ 68°F — 🔥 86°F - Colored capsule pills with icons - "Drag lines on chart" hint Phase labels: - Vertical rule marks at each phase transition on the chart - Labels at top: "Wind Down", "Fall Asleep", "Deep Sleep", etc. - Color-matched to phase (warm=orange, cool=blue, deep=dark blue) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
Sleepypod/Views/Schedule/SmartCurveView.swift (3)
328-344: Unusedlabelelement in phases tuple.The third element of each tuple (e.g.,
"+warm","cool") is never used in the rendering. Either remove it or display it to provide additional context to users.♻️ Option 1: Remove unused data
- let phases: [(String, Color, String)] = [ - ("Wind Down", Theme.warming, "+warm"), - ("Fall Asleep", Theme.cooling, "cool"), - ("Deep Sleep", Color(hex: "2563eb"), "coldest"), - ("Pre-Wake", Theme.amber, "+warm"), + let phases: [(String, Color)] = [ + ("Wind Down", Theme.warming), + ("Fall Asleep", Theme.cooling), + ("Deep Sleep", Color(hex: "2563eb")), + ("Pre-Wake", Theme.amber), ] return HStack(spacing: 12) { - ForEach(phases, id: \.0) { name, color, label in + ForEach(phases, id: \.0) { name, color in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 328 - 344, The phases array in phaseLegend currently stores tuples (String, Color, String) but the third element (label) is unused in the ForEach rendering; either remove the third element from phases and change the tuple type to (String, Color) and adapt the ForEach closure to unpack only (name, color), or use the label value in the HStack (e.g., add a Text(label) with appropriate styling). Update the phases declaration and the ForEach closure in phaseLegend accordingly so that the tuple shape and its destructuring match (or the label is rendered) to eliminate the unused value.
292-313: Remove unusedphaseRangescomputed property andPhaseRangestruct.The
phaseRangescomputed property is defined but never used. Remove both the property and the associatedPhaseRangestruct unless intended for future functionality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 292 - 313, Remove the unused PhaseRange struct and the phaseRanges computed property from SmartCurveView: delete the private struct PhaseRange and the private var phaseRanges: [PhaseRange] { ... } declaration (which references phaseColor and groups curve by \.phase) so no unused types or properties remain; if any future usage is intended, instead add a FIXME/TODO comment or mark them fileprivate/internal until they are referenced.
372-374: Remove unused state variableshealthSyncedandhealthErroror wire them to UI feedback.Both variables are set in
importFromHealth()but never read anywhere in the view's body. Either display error messages to users via an alert whenhealthErroris non-nil, or remove these unused variables entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 372 - 374, The view currently sets healthSynced and healthError inside importFromHealth() but never uses them in the view body; either remove the unused `@State` properties (healthSynced, healthError) and their assignments in importFromHealth(), or surface healthError to the UI by wiring it to an Alert (e.g. add an .alert modifier on the SmartCurveView using a Binding like .constant(healthError != nil) or a Bool computed from healthError, present the healthError string in the alert message, and clear healthError when the alert is dismissed; if you choose to keep healthSynced, bind it to visible UI feedback (a Text, badge, or disabled state) so its value is read by the view body.
🤖 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/SmartCurveView.swift`:
- Line 114: The force-unwrap of proxy.plotFrame can crash; change the
GeometryReader closure to safely unwrap proxy.plotFrame (e.g., use if let
plotFrame = proxy.plotFrame or guard let plotFrame = proxy.plotFrame else {
return EmptyView() }) and then compute let plotArea = geo[plotFrame] inside that
safe block so you never index geo with a nil rect; update references to plotArea
usage inside SmartCurveView's GeometryReader closure accordingly.
- Around line 484-487: The guard in SmartCurveView that checks
scheduleManager.schedules silently returns when nil, leaving the UI with no
feedback; modify the Save/Apply handler (where the guard using
scheduleManager.schedules and isSaving is set) to set an error state instead of
returning silently: add or reuse a boolean alert state (e.g., showErrorAlert)
and an optional errorMessage property on SmartCurveView, populate errorMessage
with a descriptive message when scheduleManager.schedules is nil, set
showErrorAlert = true and reset isSaving = false, and ensure the View presents
an alert bound to showErrorAlert to notify the user that schedules are
unavailable.
- Around line 370-376: The importFromHealth() function is implemented but never
wired into the UI; hook it up to a user action (or remove it). Locate
importFromHealth() and add a call from the intensity selector handler (reference
the intensity selector control at line ~87) or from the Apply button action
handler (reference the Apply button action method around line ~158) so that
selecting/importing triggers importFromHealth(); alternatively remove
importFromHealth() and any HealthKit-related state (e.g., healthError) if the
feature is not intended to be exposed.
---
Nitpick comments:
In `@Sleepypod/Views/Schedule/SmartCurveView.swift`:
- Around line 328-344: The phases array in phaseLegend currently stores tuples
(String, Color, String) but the third element (label) is unused in the ForEach
rendering; either remove the third element from phases and change the tuple type
to (String, Color) and adapt the ForEach closure to unpack only (name, color),
or use the label value in the HStack (e.g., add a Text(label) with appropriate
styling). Update the phases declaration and the ForEach closure in phaseLegend
accordingly so that the tuple shape and its destructuring match (or the label is
rendered) to eliminate the unused value.
- Around line 292-313: Remove the unused PhaseRange struct and the phaseRanges
computed property from SmartCurveView: delete the private struct PhaseRange and
the private var phaseRanges: [PhaseRange] { ... } declaration (which references
phaseColor and groups curve by \.phase) so no unused types or properties remain;
if any future usage is intended, instead add a FIXME/TODO comment or mark them
fileprivate/internal until they are referenced.
- Around line 372-374: The view currently sets healthSynced and healthError
inside importFromHealth() but never uses them in the view body; either remove
the unused `@State` properties (healthSynced, healthError) and their assignments
in importFromHealth(), or surface healthError to the UI by wiring it to an Alert
(e.g. add an .alert modifier on the SmartCurveView using a Binding like
.constant(healthError != nil) or a Bool computed from healthError, present the
healthError string in the alert message, and clear healthError when the alert is
dismissed; if you choose to keep healthSynced, bind it to visible UI feedback (a
Text, badge, or disabled state) so its value is read by the view body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c93fee97-33a0-4a4f-ad5c-5554e79dc603
📒 Files selected for processing (1)
Sleepypod/Views/Schedule/SmartCurveView.swift
Comment on lines
+370
to
+376
| // MARK: - HealthKit Import | ||
|
|
||
| private func importFromHealth() { | ||
| guard HKHealthStore.isHealthDataAvailable() else { | ||
| healthError = "Health data not available on this device" | ||
| return | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if importFromHealth is called anywhere
rg -n "importFromHealth" --type swiftRepository: sleepypod/ios
Length of output: 144
🏁 Script executed:
# Get more context around the function definition
sed -n '360,400p' Sleepypod/Views/Schedule/SmartCurveView.swift | cat -nRepository: sleepypod/ios
Length of output: 1975
🏁 Script executed:
# Search for any UI elements (buttons, menu items) in SmartCurveView that might relate to HealthKit
rg -n "health|Health|import|Import" Sleepypod/Views/Schedule/SmartCurveView.swift -i | head -30Repository: sleepypod/ios
Length of output: 958
🏁 Script executed:
# Check if there are any methods with common iOS patterns that might trigger this (buttons, gestures, etc.)
rg -n "@IBAction|\.onTapGesture|Button|action:|selector:" Sleepypod/Views/Schedule/SmartCurveView.swiftRepository: sleepypod/ios
Length of output: 113
🏁 Script executed:
# Check what those two buttons do
sed -n '87,95p' Sleepypod/Views/Schedule/SmartCurveView.swift | cat -nRepository: sleepypod/ios
Length of output: 604
🏁 Script executed:
sed -n '158,166p' Sleepypod/Views/Schedule/SmartCurveView.swift | cat -nRepository: sleepypod/ios
Length of output: 469
importFromHealth() is never called. The HealthKit import functionality is fully implemented but has no UI trigger—neither the intensity selector (line 87) nor the apply button (line 158) invoke this function. Either connect it to a button or menu option, or remove if no longer needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 370 - 376, The
importFromHealth() function is implemented but never wired into the UI; hook it
up to a user action (or remove it). Locate importFromHealth() and add a call
from the intensity selector handler (reference the intensity selector control at
line ~87) or from the Apply button action handler (reference the Apply button
action method around line ~158) so that selecting/importing triggers
importFromHealth(); alternatively remove importFromHealth() and any
HealthKit-related state (e.g., healthError) if the feature is not intended to be
exposed.
Comment on lines
+484
to
+487
| guard var schedules = scheduleManager.schedules else { | ||
| isSaving = false | ||
| return | ||
| } |
There was a problem hiding this comment.
Silent failure when schedules is nil.
If scheduleManager.schedules is nil, the function returns silently without any user feedback. The user sees the button change briefly but gets no indication that the operation failed.
🐛 Proposed fix to add user feedback
Task {
guard var schedules = scheduleManager.schedules else {
isSaving = false
+ // Consider showing an error state or alert here
+ Log.general.error("Cannot apply smart curve: schedules not loaded")
return
}For a better UX, consider adding an error alert state and showing it when schedules is unavailable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sleepypod/Views/Schedule/SmartCurveView.swift` around lines 484 - 487, The
guard in SmartCurveView that checks scheduleManager.schedules silently returns
when nil, leaving the UI with no feedback; modify the Save/Apply handler (where
the guard using scheduleManager.schedules and isSaving is set) to set an error
state instead of returning silently: add or reuse a boolean alert state (e.g.,
showErrorAlert) and an optional errorMessage property on SmartCurveView,
populate errorMessage with a descriptive message when scheduleManager.schedules
is nil, set showErrorAlert = true and reset isSaving = false, and ensure the
View presents an alert bound to showErrorAlert to notify the user that schedules
are unavailable.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Y-axis now auto-scales to the curve's min/max offset ±3 padding. So a 80-85°F range (offset 0 to +5) shows -3 to +8 instead of -20 to +20. The curve fills the chart height. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Chart clipped with rounded rect — curve can't overflow into text below - Y-axis uses .automatic(desiredCount: 5) so labels adapt to the current range instead of fixed -8/-4/0/4/8 values - Narrow ranges like 80-85°F now show useful increments (79, 81, 83, 85) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaced draggable overlay lines with: - Clean rule marks inside the chart (blue=min, orange=max, 1pt solid) - Centered stepper controls below: [-] ❄️ 68°F [+] | [-] 🔥 86°F [+] - 1°F per tap, haptic feedback, bounded so they can't cross Chart is now clean — no overlay gestures, no drag handling. Lines move as you tap the steppers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Chart split into two drag zones: - Drag in top half → adjusts max temp (orange line) - Drag in bottom half → adjusts min temp (blue line) - Steppers below still work for precise 1°F taps - Both update the same state — lines move in sync Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Drag: - Single rectangle covers full plot area (was two offset zones) - Uses startLocation to determine which line: bottom half = min, top half = max - Both min and max now respond to drag Layout: - Phase legend moved above temp steppers (was below Apply) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 16, 2026
This was referenced Mar 19, 2026
Merged
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This was referenced Mar 23, 2026
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
Auto-generates a science-backed temperature curve from bedtime + wake time.
User sets two times, picks an intensity (Cool/Balanced/Warm), and the app
generates a 6-phase temperature profile optimized for sleep quality.
Phases
UI
Test plan
Closes #11
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Privacy / Permissions