"Ended" on the period-end reminder records a new period instead of ending one #68

New Issue

Closed

opened 2026-08-20 21:24:13 -05:00 by null · 0 comments

null commented

2026-08-20 21:24:13 -05:00

Owner

Found by tracing the reminder pipeline at 93ec5b7.

What is true now. NotificationCopy.actionLabels (core/notifications/.../NotificationCopy.kt:123-133) gives PERIOD_END_CHECK_IN the labels ['Ended','Still going'] in Direct mode and ['Done','Not yet'] otherwise. ReminderWorker.kt:131-139 then maps button index 0 to ACTION_STARTED and index 1 to ACTION_NOT_YET — for EVERY kind, with no reference to which kind it is building. NotificationActionHandler.kt:37-49 maps ACTION_STARTED to repository.confirmPeriodStart(today) and ACTION_NOT_YET to repository.recordNotYet(today).

So when the app asks 'Is your period over?' (fired when status is DuringPeriod && dayOfPeriod >= 5, ReminderRules.kt:84-92) and the user taps Ended, the app inserts a NEW period record starting today. Tapping 'Still going' writes a censoring NotYetObservation for today. CycleRepository.setPeriodEnd (core/data/.../CycleRepository.kt:174) is unreachable from any notification.

What it costs. It corrupts the health record — a phantom period start in the middle of a real period — and every forecast built on it afterwards. The user did nothing wrong and has no way to know. This is the most damaging defect currently in the tree.

What to do. Make labels and actions ONE table: a ReminderAction { STARTED, NOT_YET, ENDED, STILL_GOING } and a ReminderButton(action, label) returned together by NotificationCopy.buttons(kind, privacy), so a label can never be paired with the wrong write again. The handler maps ENDED to setPeriodEnd on the open period (the same call TodayViewModel.setEnd makes), and STILL_GOING to no write at all. Carry the action name in the PendingIntent extra rather than deriving it from an index.

Traps. PeriodRecordSource.NOTIFICATION_CONFIRMATION exists (domain/cycle/.../Cycle.kt:6) but the handler writes MANUAL — use it. A notification already sitting in a user's shade at upgrade time carries the old action strings; parse them as unknown and write nothing rather than guessing.

Verify: a period-end check-in offers Ended and Still going and no start action in NotificationCopyTest, and ENDED closes the open period on the reminder day plus STILL_GOING writes nothing in a new app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt.

Found by tracing the reminder pipeline at 93ec5b7. **What is true now.** `NotificationCopy.actionLabels` (`core/notifications/.../NotificationCopy.kt:123-133`) gives `PERIOD_END_CHECK_IN` the labels ['Ended','Still going'] in Direct mode and ['Done','Not yet'] otherwise. `ReminderWorker.kt:131-139` then maps button index 0 to `ACTION_STARTED` and index 1 to `ACTION_NOT_YET` — for EVERY kind, with no reference to which kind it is building. `NotificationActionHandler.kt:37-49` maps `ACTION_STARTED` to `repository.confirmPeriodStart(today)` and `ACTION_NOT_YET` to `repository.recordNotYet(today)`. So when the app asks 'Is your period over?' (fired when `status is DuringPeriod && dayOfPeriod >= 5`, `ReminderRules.kt:84-92`) and the user taps **Ended**, the app inserts a NEW period record starting today. Tapping 'Still going' writes a censoring `NotYetObservation` for today. `CycleRepository.setPeriodEnd` (`core/data/.../CycleRepository.kt:174`) is unreachable from any notification. **What it costs.** It corrupts the health record — a phantom period start in the middle of a real period — and every forecast built on it afterwards. The user did nothing wrong and has no way to know. This is the most damaging defect currently in the tree. **What to do.** Make labels and actions ONE table: a `ReminderAction { STARTED, NOT_YET, ENDED, STILL_GOING }` and a `ReminderButton(action, label)` returned together by `NotificationCopy.buttons(kind, privacy)`, so a label can never be paired with the wrong write again. The handler maps ENDED to `setPeriodEnd` on the open period (the same call `TodayViewModel.setEnd` makes), and STILL_GOING to no write at all. Carry the action name in the PendingIntent extra rather than deriving it from an index. **Traps.** `PeriodRecordSource.NOTIFICATION_CONFIRMATION` exists (`domain/cycle/.../Cycle.kt:6`) but the handler writes `MANUAL` — use it. A notification already sitting in a user's shade at upgrade time carries the old action strings; parse them as unknown and write nothing rather than guessing. Verify: `a period-end check-in offers Ended and Still going and no start action` in `NotificationCopyTest`, and `ENDED closes the open period on the reminder day` plus `STILL_GOING writes nothing` in a new `app/src/test/kotlin/dev/privacyllc/period/notifications/NotificationActionHandlerTest.kt`.