null
/
Privacy-Period-Tracker
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: let the app actually stop asking 

§30 says the app stops asking "did your period start?" after a few
unanswered check-ins, and starts again next cycle. It could not.

The count was reset by a flow watching the newest confirmed start.
distinctUntilChanged only dedups within one collection, so its first
emission always passes -- and PeriodApplication starts the coordinator in
every process, including the one WorkManager spawns to run the reminder.
The count was wiped moments before the worker read it. A user who ignored
the check-ins kept being asked, daily, which is the behaviour §30 exists
to prevent and the kind people uninstall over.

The count now carries the row id of the period it was asked about and
reads as zero for any other. A new period starts it over by arithmetic
rather than by an event that has to fire at the right moment in the right
process. The reset chain is deleted outright -- there is nothing to race
and nothing for a second process to get wrong -- and the notification
handler no longer resets anything either.

An id, never a date. The rule that keeps dates out of
PeriodRecord.toString() applies to anything at rest a backup or a crash
reporter could pick up: an id says a record exists, a date says when
somebody bled. Delete My Data leaves an id matching nothing, which is
correctly no count at all.

The coordinator test that covered the deleted chain is replaced by one
that starts the coordinator twice on the same history and asserts the
tally survives -- the regression itself, rather than the machinery that
used to cause it.

Also raises the app-lock test's await budget from 5s to 30s. It went red
once in a full parallel run and passed alone: each PIN there costs a real
210,000-iteration PBKDF2 derivation, and one test asks for three. The
budget is for catching a hang, not for measuring the crypto -- and a flaky
guard is one people learn to ignore.

Proved with prove-guard, one red each: dropping the period from countFor,
and letting recordCheckIn increment across periods.

closes #70

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
null 2026-08-20 21:59:18 -05:00
parent 7f67573e8a
commit ff3cbe89ad
10 changed files with 191 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/src/main/kotlin/dev/privacyllc/period/feature/export/ExportMapping.kt
View File

@ -21,7 +21,7 @@ import dev.privacyllc.period.core.export.ExportedSettings
* - **`onboardingCompleted`** — application state, not a fact about the user.
* Nobody's archive is improved by knowing they finished a wizard.
*
* `checkInCount` is not on `UserPreferences` at all; it is stored separately and
* The check-in tally is not on `UserPreferences` at all; it is stored separately and
* is likewise app state.
*/
internal fun UserPreferences.toExportedSettings(): ExportedSettings = ExportedSettings(

6
app/src/main/kotlin/dev/privacyllc/period/notifications/NotificationActionHandler.kt
View File

@ -41,10 +41,10 @@ class NotificationActionHandler @Inject constructor(
// still opens the app; it just does not guess what she meant.
return when (ReminderAction.fromWireName(action)) {
ReminderAction.STARTED -> {
// The question is answered. Nothing resets a counter here: the
// count is stored against the period it was asked about, so this
// new start makes the old count read as zero on its own.
repository.confirmPeriodStart(today, PeriodRecordSource.NOTIFICATION_CONFIRMATION)
// The question is answered, so the app is willing to ask again
// next cycle rather than staying permanently quiet.
preferences.resetCheckIns()
true
}

44
app/src/main/kotlin/dev/privacyllc/period/notifications/ReminderCoordinator.kt
View File

@ -40,22 +40,25 @@ class ReminderCoordinator @Inject constructor(
/**
* Watch for anything that changes what a reminder should say or when.
*
* The two chains are built by [scheduleUpdates] and [checkInResets] rather
* than inline, so a test can hand them a flow that throws. Without that
* seam the only way to reach the `catch` below is a repository failure,
* and `CycleRepository` is final with an internal constructor — which is
* the right shape for a data boundary and the wrong shape for faking.
* [scheduleUpdates] is built rather than inlined so a test can hand it a
* flow that throws. Without that seam the only way to reach the `catch`
* below is a repository failure, and `CycleRepository` is final with an
* internal constructor — which is the right shape for a data boundary and
* the wrong shape for faking.
*
* There used to be a second chain here, resetting the check-in count when a
* period was confirmed. It is gone, and nothing replaced it: the count is
* now stored against the period it was asked about, so a new period starts
* it over by arithmetic instead of by an event. See
* `UserPreferencesRepository.recordCheckIn` — the chain fired its first
* value in every process, including the one WorkManager starts to run the
* reminder, which wiped the count moments before the worker read it.
*/
fun start(scope: CoroutineScope) {
scheduleUpdates(
forecastDates = repository.forecast.map { it?.mostLikelyStartDate },
preferences = preferences.preferences,
).launchIn(scope)
checkInResets(
confirmedStarts = repository.confirmedPeriods
.map { periods -> periods.maxOfOrNull { it.startDate } },
).launchIn(scope)
}
/**
@ -102,31 +105,10 @@ class ReminderCoordinator @Inject constructor(
.onEach { (_, time, enabled) -> apply(time, enabled) }
.catch { }
/**
* A confirmed period answers the question the check-ins were asking, so the
* count resets and the app is willing to ask again next cycle.
*
* Without this, somebody who ignored three check-ins once would never be
* asked again — the stopping rule would become permanent.
*
* Catches for the same reason as [scheduleUpdates], and separately from it:
* one collection failing must not stop the other, which is the whole point
* of them being two chains rather than one.
*/
internal fun checkInResets(confirmedStarts: Flow<LocalDate?>): Flow<*> =
confirmedStarts
.distinctUntilChanged()
.onEach { latest -> if (latest != null) onPeriodConfirmed(latest) }
.catch { }
private suspend fun apply(time: LocalTime, enabled: Boolean) {
if (enabled) scheduler.schedule(time) else scheduler.cancel()
}
private suspend fun onPeriodConfirmed(@Suppress("UNUSED_PARAMETER") start: LocalDate) {
preferences.resetCheckIns()
}
private fun anyEnabled(p: UserPreferences) =
p.periodApproachingEnabled || p.periodExpectedTodayEnabled || p.didItStartEnabled ||
p.periodEndCheckInEnabled || p.fertileWindowReminderEnabled || p.ovulationReminderEnabled

8
app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt
View File

@ -112,8 +112,14 @@ class LockSettingsViewModelTest {
Dispatchers.resetMain()
}
/**
* Generous on purpose. Every PIN here costs a real PBKDF2 derivation at
* 210,000 iterations, and a test that asks for three of them can pass alone
* and time out when the whole suite runs beside it. This budget is for
* catching a genuine hang, not for measuring the crypto.
*/
private fun await(predicate: suspend () -> Boolean) = runBlocking {
withTimeout(5_000) { while (!predicate()) delay(10) }
withTimeout(30_000) { while (!predicate()) delay(10) }
}
private fun pin(value: String) = value.toCharArray()

54
app/src/test/kotlin/dev/privacyllc/period/notifications/ReminderCoordinatorTest.kt
View File

@ -9,6 +9,9 @@ import dev.privacyllc.period.core.datastore.UserPreferences
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
import dev.privacyllc.period.core.notifications.ReminderScheduler
import dev.privacyllc.period.domain.prediction.PersonalPredictionEngine
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.runBlocking
@ -39,14 +42,16 @@ import java.time.Clock
class ReminderCoordinatorTest {
private lateinit var coordinator: ReminderCoordinator
private lateinit var preferences: UserPreferencesRepository
@Before fun setUp() {
val context = ApplicationProvider.getApplicationContext<Context>()
coordinator = ReminderCoordinator(
repository = CycleData.repository(context, PersonalPredictionEngine(), Clock.systemUTC()),
preferences = UserPreferencesRepository(
PreferenceDataStoreFactory.create { context.preferencesDataStoreFile("coordinator_test") },
),
)
coordinator = ReminderCoordinator(
repository = CycleData.repository(context, PersonalPredictionEngine(), Clock.systemUTC()),
preferences = preferences,
scheduler = ReminderScheduler(context),
)
}
@ -71,21 +76,44 @@ class ReminderCoordinatorTest {
).collect { }
}
@Test fun `a throwing history flow does not escape the check-in chain`() = runBlocking {
coordinator.checkInResets(
confirmedStarts = flow { throw IllegalStateException("the database could not be opened") },
).collect { }
}
/**
* The positive control, so a `catch` that swallowed everything would not
* pass the three tests above by accident. A null latest start is the
* no-history case, which reaches the collector without touching the
* scheduler or the preferences store.
* pass the two tests above by accident.
*
* A repeated key must collapse: the forecast object is rebuilt on every
* recalculation even when it says the same thing, and rescheduling on each
* of those would churn WorkManager for nothing.
*/
@Test fun `a healthy flow still reaches the collector`() = runBlocking {
var emissions = 0
coordinator.checkInResets(confirmedStarts = flowOf(null, null)).collect { emissions++ }
coordinator.scheduleUpdates(
forecastDates = flowOf(null, null),
preferences = flowOf(UserPreferences.Defaults),
).collect { emissions++ }
assertEquals("distinctUntilChanged should collapse the repeated value", 1, emissions)
}
/**
* The check-in count is not the coordinator's business any more, and this
* is the regression that made it so.
*
* The old chain reset the count when the newest confirmed start changed —
* except `distinctUntilChanged` only dedups within one collection, and
* `PeriodApplication` starts the coordinator in EVERY process, including the
* one WorkManager spawns to run the reminder. So the first emission always
* passed, the count was wiped moments before the worker read it, and §30's
* "do not nag forever" could never be reached.
*
* Starting twice on the same history now leaves the tally alone, because
* nothing is watching the history at all.
*/
@Test fun `starting twice on the same history leaves the check-in tally alone`() = runBlocking {
preferences.recordCheckIn(periodId = 7L)
preferences.recordCheckIn(periodId = 7L)
coordinator.start(CoroutineScope(Dispatchers.Unconfined))
coordinator.start(CoroutineScope(Dispatchers.Unconfined))
assertEquals(2, preferences.checkInTally.first().countFor(7L))
}
}

15
core/datastore/src/main/kotlin/dev/privacyllc/period/core/datastore/UserPreferences.kt
View File

@ -72,3 +72,18 @@ data class UserPreferences(
val Defaults = UserPreferences()
}
}
/**
* How many times the app has asked "did your period start?" — and about which
* period it was asking.
*
* The pair is the point. A bare count had to be reset when the question became
* moot, and the reset ran in every process that started the app, wiping it
* moments before the reminder worker read it. Carrying the period's id means
* the count answers itself: a count for a period that is no longer the newest
* is not a count at all.
*/
data class CheckInTally(val count: Int, val periodId: Long?) {
/** The count if it belongs to [periodId], otherwise none — see the class KDoc. */
fun countFor(periodId: Long?): Int = if (periodId != null && this.periodId == periodId) count else 0
}

38
core/datastore/src/main/kotlin/dev/privacyllc/period/core/datastore/UserPreferencesRepository.kt
View File

@ -5,6 +5,7 @@ import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.booleanPreferencesKey
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.core.intPreferencesKey
import androidx.datastore.preferences.core.longPreferencesKey
import androidx.datastore.preferences.core.stringPreferencesKey
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.catch
@ -63,13 +64,29 @@ class UserPreferencesRepository(
* the process dies. Reset when a period is confirmed, which is the event
* that makes the question moot.
*/
val checkInCount: Flow<Int> = dataStore.data
val checkInTally: Flow<CheckInTally> = dataStore.data
.catch { cause -> if (cause is IOException) emit(EMPTY) else throw cause }
.map { it[Keys.CheckInCount] ?: 0 }
.map { CheckInTally(it[Keys.CheckInCount] ?: 0, it[Keys.CheckInPeriodId]) }
suspend fun recordCheckIn() = edit { it[Keys.CheckInCount] = (it[Keys.CheckInCount] ?: 0) + 1 }
suspend fun resetCheckIns() = edit { it[Keys.CheckInCount] = 0 }
/**
* Count one check-in against the period it was asked about.
*
* A different period starts the count again, which is what the old
* `resetCheckIns()` was trying to achieve and could not: it was called from
* a flow that re-emitted its first value in **every** process — including
* the one WorkManager starts to run the reminder — so the count was wiped
* moments before the worker read it, and §30's "do not nag forever" could
* never be reached.
*
* Keying the count instead of resetting it removes the timing entirely.
* There is no chain to fire, nothing to race, and nothing to get wrong in a
* second process.
*/
suspend fun recordCheckIn(periodId: Long) = edit {
val same = it[Keys.CheckInPeriodId] == periodId
it[Keys.CheckInCount] = if (same) (it[Keys.CheckInCount] ?: 0) + 1 else 1
it[Keys.CheckInPeriodId] = periodId
}
/**
* Entitlement, mirrored from Google Play.
@ -124,6 +141,17 @@ class UserPreferencesRepository(
val DidItStart = booleanPreferencesKey("reminder_did_it_start")
val PeriodEndCheckIn = booleanPreferencesKey("reminder_period_end_check_in")
val CheckInCount = intPreferencesKey("check_in_count")
/**
* Which period the count above belongs to.
*
* A row id, never a date. The same rule that keeps dates out of
* `PeriodRecord.toString()` applies to anything at rest that a crash
* reporter or a backup could pick up: an id says a record exists, a date
* says when somebody bled. Delete My Data leaves an id matching nothing,
* which correctly reads as a count of zero.
*/
val CheckInPeriodId = longPreferencesKey("check_in_period_id")
val FertileReminder = booleanPreferencesKey("fertile_window_reminder_enabled")
val OvulationReminder = booleanPreferencesKey("ovulation_reminder_enabled")
val BiometricLock = booleanPreferencesKey("biometric_lock_enabled")

48
core/datastore/src/test/kotlin/dev/privacyllc/period/core/datastore/UserPreferencesRepositoryTest.kt
View File

@ -187,4 +187,52 @@ class UserPreferencesRepositoryTest {
assertTrue(p.incognitoLauncherEnabled)
assertTrue(p.adsRemoved)
}
// -----------------------------------------------------------------------
// The check-in tally
// -----------------------------------------------------------------------
@Test
fun `check-ins are counted against the period they were asked about`() = scope.runTest {
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 7L)
assertEquals(2, repo.checkInTally.first().countFor(7L))
}
@Test
fun `a different period starts the count again`() = scope.runTest {
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 8L)
// This is what the old resetCheckIns() was for, and it happens here by
// arithmetic rather than by an event that had to fire at the right
// moment — which it did not, in the process WorkManager starts.
assertEquals(1, repo.checkInTally.first().countFor(8L))
}
@Test
fun `a count for one period reads as nothing for another`() = scope.runTest {
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 7L)
assertEquals(0, repo.checkInTally.first().countFor(8L))
// And with no period at all — a fresh install, or after Delete My Data,
// where the stored id matches nothing that exists.
assertEquals(0, repo.checkInTally.first().countFor(null))
}
@Test
fun `the tally survives being read in another process`() = scope.runTest {
repo.recordCheckIn(periodId = 7L)
repo.recordCheckIn(periodId = 7L)
// A second repository over the same file is what the reminder worker's
// process holds. The count is state, not a session: §30's stopping rule
// is meaningless if it starts over whenever a process does.
val elsewhere = UserPreferencesRepository(store)
assertEquals(2, elsewhere.checkInTally.first().countFor(7L))
}
}

13
core/notifications/src/main/kotlin/dev/privacyllc/period/core/notifications/ReminderWorker.kt
View File

@ -76,6 +76,10 @@ class ReminderWorker @AssistedInject constructor(
val notYet = repository.notYetObservations.first()
val fertility = FertilityEstimate.from(forecast)
// The check-in count belongs to a period, not to the app. See
// UserPreferencesRepository.recordCheckIn.
val latestPeriodId = periods.maxByOrNull { it.startDate }?.id
val status = CycleStatusRules.statusFor(
periods = periods,
forecast = forecast,
@ -88,7 +92,7 @@ class ReminderWorker @AssistedInject constructor(
val decision = ReminderRules.decide(
status = status,
preferences = prefs.toReminderPreferences(),
checkInsSoFar = preferences.checkInCount.first(),
checkInsSoFar = preferences.checkInTally.first().countFor(latestPeriodId),
fertility = fertility,
today = today,
)
@ -115,8 +119,9 @@ class ReminderWorker @AssistedInject constructor(
contentIntent = openApp(),
)
// Counted, so the next wake-up sees a number past the limit and
// sends nothing.
preferences.recordCheckIn()
// sends nothing. Against the period it was asked about, so the
// next period starts the count over on its own.
latestPeriodId?.let { preferences.recordCheckIn(it) }
return Result.success()
}
@ -147,7 +152,7 @@ class ReminderWorker @AssistedInject constructor(
if (decision.kind == ReminderKind.DID_IT_START ||
decision.kind == ReminderKind.PERIOD_EXPECTED_TODAY
) {
preferences.recordCheckIn()
latestPeriodId?.let { preferences.recordCheckIn(it) }
}
return Result.success()
}

21
docs/architecture/README.md
View File

@ -157,6 +157,27 @@ health app a crash mid-write is adjacent to losing what the user just entered,
and a message they can read beats a process that vanished. The message carries
the exception *type* and never a record's contents — §45.
### The check-in count belongs to a period, not to the app
§30 says the app must stop asking "did your period start?" after a few
unanswered check-ins, and start again next cycle. That was a counter plus a
reset, and the reset ran in a flow watching the newest confirmed start —
`distinctUntilChanged`, whose first emission always passes, in a coordinator
`PeriodApplication` starts in **every** process. Including the one WorkManager
spawns to run the reminder, moments before the worker reads the count. The
stopping rule could never be reached.
The count is now stored with the row id of the period it was asked about
(`check_in_period_id`), and reads as zero for any other. A new period starts it
over by arithmetic instead of by an event that has to fire at the right moment
in the right process; there is no chain to race and nothing for a second process
to get wrong. Delete My Data leaves an id matching nothing, which is correctly
no count at all.
An id, never a date — the rule that keeps dates out of `PeriodRecord.toString()`
applies to anything at rest a backup or a crash reporter could pick up. An id
says a record exists; a date says when somebody bled.
### One forecast stands at a time, and backfill is not a prediction
Two rules about `prediction_records` that are easy to get wrong and expensive to