Skip to content

Add a recompute action for deleted sleep windows - #526

Merged
ryanbr merged 3 commits into
ryanbr:mainfrom
kavemang:fix/recompute-deleted-sleep-515
Jul 17, 2026
Merged

Add a recompute action for deleted sleep windows#526
ryanbr merged 3 commits into
ryanbr:mainfrom
kavemang:fix/recompute-deleted-sleep-515

Conversation

@kavemang

Copy link
Copy Markdown

Summary

  • show durable deleted-sleep windows on the Android Sleep screen after the transient Undo banner expires
  • add a Recompute this night action that clears only the selected deletion tombstone and immediately reruns sleep detection/scoring
  • read markers across the active-strap and canonical source union so the escape hatch survives a strap remove/re-add
  • keep the marker visible and skip the misleading rescore when its database removal fails

This addresses the deleted-night dead end described in item 4 of #515. Normal deletion behavior is unchanged: detected nights remain suppressed until the user explicitly chooses to recompute one.

Validation

  • cd android && ./gradlew :app:testFullDebugUnitTest
  • focused deleted-sleep guard, tombstone-policy, and recompute control-flow tests

@kavemang
kavemang marked this pull request as ready for review July 16, 2026 19:21
@kavemang
kavemang marked this pull request as draft July 16, 2026 19:29
@kavemang
kavemang marked this pull request as ready for review July 16, 2026 20:04
@ryanbr
ryanbr merged commit 96aee18 into ryanbr:main Jul 17, 2026
1 check passed
ryanbr added a commit that referenced this pull request Jul 17, 2026
Numbers are main's current 292/203 +1, matching the previous staging commit's
pattern, so the .staging app upgrades cleanly over the last testing build (290)
and stays above main's released high-water.

Carries everything merged since the 9.0.1 release:
 - Workouts read HR under the recording strap, not a hardcoded id (#512 Kotlin,
   #513 Swift twin)
 - Oura 0x60/0x80 IBI decoders corrected to the ring's real byte layout (#511)
 - Whoop5Config enable_sig12 corrected to ASCII '1' (#522)
 - Sleep time edits save as one validated window (#525)
 - "Recompute this night" for deleted sleep windows (#526)
 - Stress marker values stay readable (#507)
 - Faster history sync + faster Bluetooth link, both EXPERIMENTAL and default-off
   (#536/#537/#538) - the whole point of this build is field data on those
 - i18n changelog-title fix + main-push audit (#514/#535)

versionName stays 9.0.2 here for the staging identity only; the real 9.0.2
release still bumps main separately (and must be raised past 293 first).
ryanbr pushed a commit that referenced this pull request Jul 20, 2026
Add a Hide action for persistent deleted-sleep windows on the Android Sleep screen. A new managementVisible flag (additive Room MIGRATION_21_22, default 1) separates the recompute-list view from the detector tombstone: hiding a row only filters the management read (dismissedSleepsUnion), while the detector's unfiltered read keeps suppressing the deleted sleep. Localized de/es/fr/zh + a11y; migration + read-separation tests. iOS tombstones live in UserDefaults, so no GRDB twin. Related #515/#526.
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
* Add deleted sleep recompute action

* Retrigger i18n checks

* Localize deleted sleep recompute UI

---------

Co-authored-by: kavemang <kavemang@users.noreply.github.com>
DX23876 pushed a commit to DX23876/noop that referenced this pull request Jul 22, 2026
Add a Hide action for persistent deleted-sleep windows on the Android Sleep screen. A new managementVisible flag (additive Room MIGRATION_21_22, default 1) separates the recompute-list view from the detector tombstone: hiding a row only filters the management read (dismissedSleepsUnion), while the detector's unfiltered read keeps suppressing the deleted sleep. Localized de/es/fr/zh + a11y; migration + read-separation tests. iOS tombstones live in UserDefaults, so no GRDB twin. Related ryanbr#515/ryanbr#526.
ryanbr added a commit that referenced this pull request Jul 30, 2026
The caffeine window (#526) already stores exactly what a HealthKit caffeine
sample carries — a timestamp and an optional mg — so a coffee logged in
another app can feed the decay estimate instead of being typed in twice.

A SAMPLE query, not the day-bucketed one water uses: the estimate decays each
intake from its own time, so a day total would collapse a 7am coffee and a 9pm
one into a number that answers no question the card asks.

Dedup is harder than water's. Water is one day total that can simply be
replaced; caffeine is a list of individual events. So an imported intake
carries its HealthKit sample UUID in a new optional `externalId`, and a sync
replaces the imported SUBSET wholesale, leaving hand-logged intakes alone.
That keeps a re-import idempotent instead of logging a second copy of every
coffee, and makes an intake deleted in the source app disappear here too.
The field is optional so existing stored JSON still decodes, with every
existing intake correctly reading back as hand-logged.

Imported intakes are not editable here: remove/clearAll skip them and the UI
offers no control, because the next sync would re-read the same window and
bring the entry straight back. Remove it where it was logged.

The store gains a `shared` instance — the card used to own its own, which was
fine while every write came from the card, but two instances over one
UserDefaults key would leave the card publishing a stale array while the
imported intakes sat invisible on disk. Follows the LiquidPowerMonitor
pattern already used for @mainactor ObservableObject singletons.

ANDROID IS DELIBERATELY NOT MATCHED. Health Connect has no caffeine-only
scope: caffeine is a field on NutritionRecord behind READ_NUTRITION, which
grants the user's entire food log. Apple exposes .dietaryCaffeine as its own
narrow type. Asking every Android user for their whole nutrition history to
read one number is not a trade this app should make quietly, so the manifest
is untouched and the gap is documented instead.

Tests: 15 in StrandTests covering idempotency, the vanished-sample case, the
backward-compatible decode, and the not-editable rules. No CI runs them —
app-build.yml is disabled and StrandTests executes nowhere.
ryanbr added a commit that referenced this pull request Jul 30, 2026
The caffeine window (#526) already stores exactly what a HealthKit caffeine
sample carries — a timestamp and an optional mg — so a coffee logged in
another app can feed the decay estimate instead of being typed in twice.

A SAMPLE query, not the day-bucketed one water uses: the estimate decays each
intake from its own time, so a day total would collapse a 7am coffee and a 9pm
one into a number that answers no question the card asks.

Dedup is harder than water's. Water is one day total that can simply be
replaced; caffeine is a list of individual events. So an imported intake
carries its HealthKit sample UUID in a new optional `externalId`, and a sync
replaces the imported SUBSET wholesale, leaving hand-logged intakes alone.
That keeps a re-import idempotent instead of logging a second copy of every
coffee, and makes an intake deleted in the source app disappear here too.
The field is optional so existing stored JSON still decodes, with every
existing intake correctly reading back as hand-logged.

Imported intakes are not editable here: remove/clearAll skip them and the UI
offers no control, because the next sync would re-read the same window and
bring the entry straight back. Remove it where it was logged.

The store gains a `shared` instance — the card used to own its own, which was
fine while every write came from the card, but two instances over one
UserDefaults key would leave the card publishing a stale array while the
imported intakes sat invisible on disk. Follows the LiquidPowerMonitor
pattern already used for @mainactor ObservableObject singletons.

ANDROID IS DELIBERATELY NOT MATCHED. Health Connect has no caffeine-only
scope: caffeine is a field on NutritionRecord behind READ_NUTRITION, which
grants the user's entire food log. Apple exposes .dietaryCaffeine as its own
narrow type. Asking every Android user for their whole nutrition history to
read one number is not a trade this app should make quietly, so the manifest
is untouched and the gap is documented instead.

Tests: 15 in StrandTests covering idempotency, the vanished-sample case, the
backward-compatible decode, and the not-editable rules. No CI runs them —
app-build.yml is disabled and StrandTests executes nowhere.
ryanbr added a commit that referenced this pull request Jul 30, 2026
The caffeine window (#526) already stores exactly what a HealthKit caffeine
sample carries — a timestamp and an optional mg — so a coffee logged in
another app can feed the decay estimate instead of being typed in twice.

A SAMPLE query, not the day-bucketed one water uses: the estimate decays each
intake from its own time, so a day total would collapse a 7am coffee and a 9pm
one into a number that answers no question the card asks.

Dedup is harder than water's. Water is one day total that can simply be
replaced; caffeine is a list of individual events. So an imported intake
carries its HealthKit sample UUID in a new optional `externalId`, and a sync
replaces the imported SUBSET wholesale, leaving hand-logged intakes alone.
That keeps a re-import idempotent instead of logging a second copy of every
coffee, and makes an intake deleted in the source app disappear here too.
The field is optional so existing stored JSON still decodes, with every
existing intake correctly reading back as hand-logged.

Imported intakes are not editable here: remove/clearAll skip them and the UI
offers no control, because the next sync would re-read the same window and
bring the entry straight back. Remove it where it was logged.

The store gains a `shared` instance — the card used to own its own, which was
fine while every write came from the card, but two instances over one
UserDefaults key would leave the card publishing a stale array while the
imported intakes sat invisible on disk. Follows the LiquidPowerMonitor
pattern already used for @mainactor ObservableObject singletons.

ANDROID IS DELIBERATELY NOT MATCHED. Health Connect has no caffeine-only
scope: caffeine is a field on NutritionRecord behind READ_NUTRITION, which
grants the user's entire food log. Apple exposes .dietaryCaffeine as its own
narrow type. Asking every Android user for their whole nutrition history to
read one number is not a trade this app should make quietly, so the manifest
is untouched and the gap is documented instead.

Tests: 15 in StrandTests covering idempotency, the vanished-sample case, the
backward-compatible decode, and the not-editable rules. No CI runs them —
app-build.yml is disabled and StrandTests executes nowhere.
ryanbr added a commit that referenced this pull request Jul 31, 2026
* Caffeine: import intakes from Apple Health (#949)

The caffeine window (#526) already stores exactly what a HealthKit caffeine
sample carries — a timestamp and an optional mg — so a coffee logged in
another app can feed the decay estimate instead of being typed in twice.

A SAMPLE query, not the day-bucketed one water uses: the estimate decays each
intake from its own time, so a day total would collapse a 7am coffee and a 9pm
one into a number that answers no question the card asks.

Dedup is harder than water's. Water is one day total that can simply be
replaced; caffeine is a list of individual events. So an imported intake
carries its HealthKit sample UUID in a new optional `externalId`, and a sync
replaces the imported SUBSET wholesale, leaving hand-logged intakes alone.
That keeps a re-import idempotent instead of logging a second copy of every
coffee, and makes an intake deleted in the source app disappear here too.
The field is optional so existing stored JSON still decodes, with every
existing intake correctly reading back as hand-logged.

Imported intakes are not editable here: remove/clearAll skip them and the UI
offers no control, because the next sync would re-read the same window and
bring the entry straight back. Remove it where it was logged.

The store gains a `shared` instance — the card used to own its own, which was
fine while every write came from the card, but two instances over one
UserDefaults key would leave the card publishing a stale array while the
imported intakes sat invisible on disk. Follows the LiquidPowerMonitor
pattern already used for @mainactor ObservableObject singletons.

ANDROID IS DELIBERATELY NOT MATCHED. Health Connect has no caffeine-only
scope: caffeine is a field on NutritionRecord behind READ_NUTRITION, which
grants the user's entire food log. Apple exposes .dietaryCaffeine as its own
narrow type. Asking every Android user for their whole nutrition history to
read one number is not a trade this app should make quietly, so the manifest
is untouched and the gap is documented instead.

Tests: 15 in StrandTests covering idempotency, the vanished-sample case, the
backward-compatible decode, and the not-editable rules. No CI runs them —
app-build.yml is disabled and StrandTests executes nowhere.

* Caffeine: stable ids for imported intakes, and don't wipe them on a read error

Two defects found re-reviewing the PR.

The intake id was freshly minted on every sync. CaffeineIntake.id defaults to
UUID(), and the bridge rebuilds every intake from the HealthKit samples each
time, so the same coffee got a new identity on every pass. That defeated
replaceImported's "nothing changed" guard (a pointless JSON rewrite and
republish on every sync) and gave ForEach an entirely new set of rows to
rebuild each time. The sample's own uuid is stable, so use it.

My own test could not have caught this: it reused one CaffeineIntake value for
both imports, so the ids matched trivially. It now builds a fresh intake per
import, the way the bridge does, plus an explicit test that two intakes built
from one sample are equal.

Second, collectCaffeine discarded the query error, so a failed read returned
an empty array and replaceImported deleted every imported intake — the same
class of bug as the hydration one in the previous commit, and worse here
because the caller's whole job is to replace the set. It now returns nil on
failure, and only a genuine empty result clears anything.

* Caffeine: the permission note covers caffeine too, now that it is read (#949)

* Treat a failed sample cast as a failed read, not an empty night (#949)

The last instance of the pattern that has bitten four times today: the cast
used `?? []`, so an unexpected sample type would read as "no caffeine" — and
because the caller replaces the imported set wholesale, that would silently
delete every imported intake. Vanishingly unlikely on a query typed to a
quantity type, but it is the same failure shape as the error check sitting
beside it, so it now shares that check.
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