Skip to content

fix(sleep): persist import edits and recalculate sleep debt#838

Open
DX23876 wants to merge 3 commits into
ryanbr:mainfrom
DX23876:fix-ios-sleep-edit-save
Open

fix(sleep): persist import edits and recalculate sleep debt#838
DX23876 wants to merge 3 commits into
ryanbr:mainfrom
DX23876:fix-ios-sleep-edit-save

Conversation

@DX23876

@DX23876 DX23876 commented Jul 26, 2026

Copy link
Copy Markdown

What this PR does

  • Persists a manual sleep-time edit even when the visible imported session belongs to the canonical source after a strap re-add.
  • Rebuilds the edited wake-day sleep fields from the stored, re-clipped stages so the corrected asleep duration reaches the Sleep screen immediately.
  • Preserves an imported sleep_debt_min baseline after an edit and adjusts it by the exact asleep-duration delta: max(0, importedDebt + originalImportedAsleep - editedAsleep).
  • Adds the same stale-debt correction to Android for both the Sleep Debt tile and its trend.
  • Keeps the owner-resolution change Swift-only intentionally: Android SleepSession already carries its owning deviceId, so that failure mode cannot occur there.
  • Falls back to the existing need-minus-asleep approximation when an older import has no original duration to preserve.

Android behavior changes only where required for sleep-debt parity. No BLE, protocol, migration, or generated-project behavior changed.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

  • macOS build and StrandTests passed in GitHub Actions on macOS 15. The local equivalent passed 967 tests, with 1 expected Xiaomi fixture skip.
  • The iOS NOOPiOS simulator build passed in the same GitHub Actions run on macOS 26.
  • All eight Swift package CI suites passed in GitHub Actions: WhoopProtocol, OuraProtocol, PolarProtocol, WhoopStore, StrandAnalytics, StrandImport, StrandDesign, and NoopLocalAccess.
  • python3 Tools/i18n_audit.py --ci upstream/main passed for Android and all Apple catalogs.
  • git diff --check passed.
  • Android assembleFullDebug passed in GitHub Actions. Its unit-test compilation is currently blocked by three untouched DeviceRegistryDao test fakes that predate this PR and do not implement newly required DAO members; this PR does not mix that independent test-harness repair into the sleep fix.

Checklist

  • macOS app build and StrandTests pass.
  • iOS app build passes.
  • Swift package tests pass for all packages in the CI matrix.
  • Android unit tests pass; blocked by an existing test-harness compilation issue outside this PR.
  • No new build warnings from changed files.
  • UI changes use existing data and design-system conventions.
  • No hardcoded hex frame bytes; protocol facts remain in the schema / decoders.
  • Follows the conventions in docs/CONTRIBUTING.md.
  • No generated output (Strand.xcodeproj/) or secrets/keystores committed.

@DX23876
DX23876 marked this pull request as ready for review July 26, 2026 18:14
@DX23876
DX23876 marked this pull request as draft July 26, 2026 18:27
@DX23876 DX23876 changed the title fix(sleep): persist edits for canonical sleep history fix(sleep): persist import edits and recalculate sleep debt Jul 26, 2026
@DX23876
DX23876 marked this pull request as ready for review July 26, 2026 18:40

@ryanbr ryanbr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good diagnosis — the CachedSleepSession-has-no-deviceId root cause is right, and resolving the owner against the same union the reads use is the correct shape.

  1. Parity. Android has the same stale-debt bug, at SleepModelLogic.kt:194 (imported.debtMin[d.day] ?: fallback) and again at :209 for trendDebtHours. An edited night there still reads the export's original debt. Needs the twin in this PR, or a stated reason not to.

  2. Say that the owner-resolution half is Swift-only on purpose. Android's updateSleepSessionTimes takes a SleepSession that carries its own deviceId, so that bug cannot exist there. Worth a line in the PR body, otherwise the missing Android change reads as an oversight rather than a deliberate one.

  3. Question, not a blocker: on an edited day you drop the imported debt entirely and fall back to need − asleep. Would adjusting the imported value by the duration delta keep more of the export's accuracy than replacing it with our approximation?

Also worth noting no CI covers Strand/ or StrandTests, so your local run is the only verification this has.

@DX23876
DX23876 marked this pull request as draft July 26, 2026 22:23
@DX23876

DX23876 commented Jul 26, 2026

Copy link
Copy Markdown
Author

Thanks, that was a helpful catch. I pushed the Android twin and changed the edited-day calculation to keep the imported baseline and apply the asleep-duration delta instead of replacing it with need - asleep.

Both the Android tile and trend now use:

max(0, importedDebt + originalImportedAsleep - editedAsleep)

The owner-resolution half remains Swift-only intentionally: Android's updateSleepSessionTimes receives a SleepSession that already has its deviceId, so it cannot lose the row owner in that path. I added that context to the PR description as well.

For verification, the macOS build and StrandTests, the iOS build, and all Swift package suites passed in Actions on my fork. Android assembleFullDebug also passes. Its unit-test compilation is currently stopped by three unchanged DeviceRegistryDao test fakes that no longer implement newly required DAO members; I documented that separately rather than mixing an unrelated test-harness repair into this sleep fix.

@DX23876
DX23876 marked this pull request as ready for review July 26, 2026 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants