fix(sleep,workouts): delete native strength sessions, honor Liquid setting on the Sleep hero, tier the "may be incomplete" warning - #24
Conversation
…tting on the Sleep hero, and tier the "may be incomplete" warning by how short the night actually reads Three unrelated reports from grilling this fork's outstanding issues, bundled onto one branch because they touch the same review pass and none needed its own migration. **Native strength sessions were visible but undeletable (ryanbr#2278-adjacent).** `deleteWorkout` swept strap namespaces by natural key, but a native row is a READ-TIME projection of `trainingWorkoutNative` with no `workout` row behind it — the sweep matched nothing, and the session reappeared on reload every time. `deleteNativeWorkoutRow` now removes, in order: the native workout (exercises/sets cascade via the FK), its `trainingSessionLink`, and the legacy manual "Strength Training" recording `hidingLegacyStrengthRecordings` was hiding as its twin — deleting the native alone would have uncovered that recording and made the delete look ignored again. `isLegacyStrengthTwin` also gained a zero-length fallback: a start/stop inside the same second has no "half" to overlap under the existing ratio test, so its twin was never recognized and stayed on screen after one copy was deleted. Everything else DRATH reported turned out to already be fixed by today's upstream sync (ryanbr#2286 deletes from every strap namespace the list reads; ryanbr#2287 discards sub-minute live sessions at save and floors manual entry the same way). The strength-specific gap above is what neither covers: a native session isn't in any `workout` namespace at all. Confirmed no duration floor was missing on the two strength-save paths either — `NativeWorkoutEngine.complete` and `LiftSessionView.save` already refuse to write anything with zero completed sets, regardless of elapsed time, so an accidental instant start already saves nothing to delete. **Sleep hero stayed on the liquid gauge with Liquid Design off.** `SleepView.restHero` drew `LiquidScoreGauge` unconditionally; every other screen already read `TodayDashboardStyle.storageKey` to fall back to Classic. The hero now reads the same setting: `.liquid` keeps the vessel gauge and night scene, every other style draws `GlowRing` on a plain `NoopCard` like Classic Today's ring, exactly as the rest of the Sleep tab already does. **"May be incomplete" fired on nights that weren't.** The badge was driven by `stagingSparse` alone, which flags thin MOTION coverage, not a short night — on a sample history 57 of 92 computed nights were sparse, including 10.5h/12h/12.75h nights. `sparseStagingNote` now tiers it: prominent (badge + the original copy) only when the night reads under 70% of the wearer's 30-day typical (or under 4h with fewer than 5 scored nights to average), a quiet footnote otherwise. When the partial-timeline note (ryanbr#1716, a MEASURED hole in the timeline) is already shown, sparse steps down to the footnote rather than stacking a second warning beside the stronger, attributable one. `showsMotionStrip` also stops captioning "No movement detail" on imported/pre-migration nights (`stagingSparse == nil` on every block) — those never carried per-epoch motion to begin with, so the line was noise, not a fact about the night. Two smaller fixes surfaced by the same pass: `SleepModel.alignedMotion` trims a fragment's persisted motion trace to the window a hand-edited night now shows (it was gridded from the DETECTED start and drawn unclipped, so a moved onset/wake left restless bursts under the wrong stage bands) — no padding on an onset moved earlier, since there's no real sample for that time and zeros would draw a fabricated still sleeper. And a new always-on diagnostic line (once per session per launch) logs when a kept session staged with no per-epoch motion at all, naming duration, `stagingSparse` and whether Motion-aware Wake was on, without asserting a cause — the reporter's zero-motion night couldn't be explained from the code alone and needs a Test Centre log to triage further. Mirrored in both hosts that render this note (`SleepView`, `StagesCard`'s read-only `StageDetailView`) off the same shared static helpers so the two screens can't disagree about a night's tier. Verified: `swift test` green in WhoopStore (755, incl. the new `deleteTrainingSessionLinks` scoping test) and StrandDesign (147, 5 pre-existing locale-dependent failures unrelated to this diff — confirmed present on the merge-commit baseline before these changes too); `xcodebuild test` green for StrandTests (3405 tests, 0 failures) on both this diff and after a hard reset to confirm nothing was lost mid-session; `xcodebuild build` green for NOOPiOS (this platform's default CI does not build app targets, see CLAUDE.md). `Tools/i18n_audit.py --ci main` exits 0 for the one new string (the sparse footnote, identical in both hosts, added to all nine languages in the catalog and `Tools/translations/`). `doc_comment_lint` clean. Not tested on a real strap: nothing here touches the BLE/CoreBluetooth path. Analysis migration required: no — no scoring formula, analytics window, or persisted-value meaning changed; `stagingSparse` is read the same way it always was, only presented differently. Separately found and NOT part of this change: `Tools/tests/test_parity_governance_acceptance.py` and `Tools/tests/test_rr_legacy_preservation_contract.py` reference files under `android/`, which this fork removed on 2026-08-14 per docs/FORK_GUIDE.md, and the parity-governance baseline appears stale after today's upstream sync — both break the active `tools-python.yml` CI gate already on the sync/upstream-2026-09-17 merge commit, before any of this branch's changes. Flagging for a separate pass rather than bundling an unrelated fix here.
|
Landed on One follow-up the sync required: upstream ryanbr#2296 replaced Verified on the merged state: StrandTests 3414 tests / 0 failures, WhoopStore 755 / 0 failures, NOOPiOS builds and is installed on a device, |
Stacked on #23 (upstream sync) — this PR's diff is only the commit on top of that merge.
Three reports from a grilling session, bundled onto one branch (same review pass, no migration on any of them).
Native strength sessions were visible but undeletable.
deleteWorkoutswept strap namespaces by natural key, but a native row is a read-time projection oftrainingWorkoutNativewith noworkoutrow behind it — the sweep matched nothing and the session reappeared on reload.deleteNativeWorkoutRownow removes the native workout (exercises/sets cascade via FK), itstrainingSessionLink, and the legacy manual "Strength Training" twinhidingLegacyStrengthRecordingswas hiding — deleting the native alone would have uncovered that twin and made the delete look ignored again.isLegacyStrengthTwinalso gained a zero-length fallback (a start/stop inside the same second has no "half" to overlap under the ratio test).Everything else in the original report (deletion touching only the active strap, sub-minute sessions) turned out to already be fixed by #23's upstream sync (ryanbr#2286, ryanbr#2287). Confirmed no duration floor was missing on the strength-save paths either — both already refuse zero-completed-set sessions regardless of elapsed time.
Sleep hero stayed on the liquid gauge with Liquid Design off.
SleepView.restHerodrewLiquidScoreGaugeunconditionally. It now readsTodayDashboardStyle.storageKeylike every other screen:.liquidkeeps the vessel gauge, every other style drawsGlowRingon a plain card like Classic Today."May be incomplete" fired on nights that weren't short. The badge was driven by
stagingSparsealone (thin motion coverage), not by whether the night actually read short — on a sample history 57/92 computed nights were sparse, including 10.5h/12h/12.75h nights.sparseStagingNotenow tiers it: prominent only when the night is under 70% of the 30-day typical (or under 4h with fewer than 5 scored nights), a quiet footnote otherwise, stepping down when the stronger partial-timeline note (ryanbr#1716) is already shown.showsMotionStripstops captioning "No movement detail" on imported/pre-migration nights that never carried per-epoch motion.Two smaller fixes from the same pass:
SleepModel.alignedMotiontrims a fragment's persisted motion trace to the window a hand-edited night now shows (previously drawn unclipped from the detected start, so a moved onset/wake put restless bursts under the wrong stage bands). A new always-on diagnostic line (once per session per launch) logs when a kept session staged with no per-epoch motion, naming duration/stagingSparse/Motion-aware-Wake state without asserting a cause — needs a Test Centre log from a reporter to triage further.Mirrored in both hosts that render the note (
SleepView,StagesCard) off shared static helpers so they can't disagree.Verified:
swift testgreen — WhoopStore 755 (incl. newdeleteTrainingSessionLinkstest), StrandDesign 147 (5 pre-existing locale-dependent failures confirmed present before this diff too).xcodebuild testgreen for StrandTests, 3405 tests, 0 failures — run twice, including after a mid-sessiongit reset --hardrecovery, to confirm nothing was lost.xcodebuild buildgreen for NOOPiOS (this fork's default CI doesn't build app targets).i18n_audit.py --ciexits 0 for the one new string (all nine languages).doc_comment_lintclean. Not tested on a real strap — nothing here touches BLE.Analysis migration required: no — no scoring formula, analytics window, or persisted-value meaning changed;
stagingSparseis read exactly as before, just presented differently.Separately found, not part of this change:
Tools/tests/test_parity_governance_acceptance.pyandTools/tests/test_rr_legacy_preservation_contract.pyreferenceandroid/paths this fork removed on 2026-08-14, and the parity-governance baseline reads stale after the sync. Correction to an earlier note here: these do NOT breaktools-python.yml's activelinux-capturejob (its "Run Tools/ tests" step only runs the six top-levelTools/test_*.pymodules, neverTools/tests/) — they break the separate, undocumented.github/workflows/parity-governance.yml. See #25, which retires that whole stack. #23 also needed two small, unrelated CI fixes of its own (12 new untranslated literals from upstream's own sync content, and the same two-line test fix #25 makes, cherry-picked since #25 is base-mainand independent).🤖 Generated with Claude Code