diff --git a/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleData.kt b/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleData.kt index 491524a..293e0b1 100644 --- a/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleData.kt +++ b/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleData.kt @@ -3,7 +3,6 @@ package dev.privacyllc.period.core.data import android.content.Context import androidx.room.Room import dev.privacyllc.period.core.database.PeriodDatabase -import dev.privacyllc.period.domain.prediction.BaselinePredictionEngine import dev.privacyllc.period.domain.prediction.PredictionEngine import java.time.Clock @@ -22,9 +21,23 @@ import java.time.Clock */ object CycleData { + /** + * [engine] has no default, deliberately. + * + * Which engine ships is one decision, and it is made in one line — + * `DataModule.predictionEngine()`. A default here was a second, silent place + * the same decision could be taken: a caller who omitted the argument would + * compile cleanly and quietly get `BaselinePredictionEngine`, the prototype + * §11 names as an unacceptable final engine and which this product retired + * in Batch 02. Nobody was relying on it; requiring the argument means nobody + * can start. + * + * [clock] keeps its default for the opposite reason: the system clock is not + * a product choice, and §50's timezone cases need to override it in tests. + */ fun repository( context: Context, - engine: PredictionEngine = BaselinePredictionEngine(), + engine: PredictionEngine, clock: Clock = Clock.systemDefaultZone(), ): CycleRepository = CycleRepository(database(context), engine, clock) diff --git a/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleRepository.kt b/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleRepository.kt index 491f97a..718343f 100644 --- a/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleRepository.kt +++ b/core/data/src/main/kotlin/dev/privacyllc/period/core/data/CycleRepository.kt @@ -165,7 +165,7 @@ class CycleRepository internal constructor( ) scoreOutstanding(startDate) - notYetDao.deleteBefore(startDate.plusDays(1)) + clearResolvedNotYet(startDate) snapshotForecast(basedOnPeriodId = id) PeriodWriteResult.Added(id) } @@ -210,12 +210,26 @@ class CycleRepository internal constructor( updatedAt = clock.instant(), ), ) + if (startDate != existing.startDate) rescoreAgainst(existing.startDate, startDate) snapshotForecast(basedOnPeriodId = id) PeriodWriteResult.Updated } + /** + * Remove a record, and with it any score that was a fact about it. + * + * Delete the period and the score's ground truth goes with it: keeping the + * row would feed §16's figures and the engine's error window a measurement + * about a start the user has withdrawn. To move a period *without* losing + * its sample, edit it — [editPeriod] re-scores. Delete-and-re-add cannot, + * deliberately: the app has been told the first one never happened. + */ suspend fun deletePeriod(id: Long) = db.inTransaction { + val existing = periodDao.byId(id) ?: return@inTransaction periodDao.deleteById(id) + for (snapshot in predictionDao.scoredAgainst(existing.startDate)) { + predictionDao.deleteById(snapshot.id) + } snapshotForecast(basedOnPeriodId = null) } @@ -253,6 +267,55 @@ class CycleRepository internal constructor( // Internals // ----------------------------------------------------------------------- + /** + * Clear the "not yet" observations this confirmed start actually resolved. + * + * The newest confirmed start resolves the standing question outright, so all + * of them go — **including any dated after a retroactively logged start**, + * which is exactly what a date-bounded clear leaves behind. Those would + * otherwise censor the next cycle's forecast with answers about a cycle that + * is already resolved: a confidence penalty + * (`PersonalPredictionEngine`'s `notYetPenalty`) and a window floor, both + * earned by nothing. + * + * A backfill into the middle of history resolves nothing about the standing + * question, so the live observations stay and only strays on or before the + * backfilled date are cleared, as before. + */ + private suspend fun clearResolvedNotYet(startDate: LocalDate) { + // Non-empty: the record for startDate was inserted moments ago. + val isNewestStart = periodDao.confirmedStartDates().max() == startDate + if (isNewestStart) notYetDao.deleteAll() else notYetDao.deleteBefore(startDate.plusDays(1)) + } + + /** + * The outcome moved, so the scores recorded against it move with it. + * + * Symmetric by construction: `predictedStartDate` is immutable, so a + * correction worsens the figure as readily as it improves one. What an edit + * must never do is smuggle a score past [scoreOutstanding]'s backfill rule, + * so the same test gates both — if the start now falls before the day this + * forecast's lineage began, the forecast was made after the period had (per + * the corrected record) already started, and a score that would never have + * been recorded in the first place is retracted rather than recomputed. + * + * Runs before [snapshotForecast], so the next forecast is built from + * corrected errors rather than the ones it is about to replace. + */ + private suspend fun rescoreAgainst(previousStart: LocalDate, correctedStart: LocalDate) { + for (snapshot in predictionDao.scoredAgainst(previousStart)) { + val lineageBegan = snapshot.generatedAt.atZone(clock.zone).toLocalDate() + if (lineageBegan.isAfter(correctedStart)) { + predictionDao.deleteById(snapshot.id) + } else { + val error = abs( + snapshot.predictedStartDate.toEpochDay() - correctedStart.toEpochDay(), + ).toInt() + predictionDao.score(snapshot.id, correctedStart, error) + } + } + } + /** * Record what actually happened against the forecast that was standing. * @@ -268,8 +331,15 @@ class CycleRepository internal constructor( * getting better" with figures the app made up about itself. * * So a forecast is only scored when the confirmed start falls on or after - * the day the forecast was made. Otherwise it survives untouched, still - * waiting for the period it is actually about. + * the day its **lineage** began — see [snapshotForecast], which carries that + * date across every revision of the standing forecast. Otherwise nothing is + * scored now: the snapshot that replaces this one inherits the same origin, + * so the standing question keeps its original date and is still waiting for + * the period it is actually about. + * + * This comment used to claim the snapshot "survives untouched", which was + * false — the very next statement in [confirmPeriodStart] replaces it. The + * carry is what makes the intent true rather than the comment wrong. */ private suspend fun scoreOutstanding(actualStart: LocalDate) { val standing = predictionDao.unscored().firstOrNull() ?: return @@ -291,16 +361,61 @@ class CycleRepository internal constructor( * forecast nobody was looking at when the period arrived is not a wrong * forecast, and counting it as one would let a single cycle contribute * several errors to the accuracy figures. + * + * ## A replacement is a revision, not a new question + * + * Every snapshot since the last scoring answers one question: *when does the + * period after the latest confirmed start begin?* A "Not yet", an edit or a + * delete revises the answer; none of them asks something new. So the + * replacement inherits the **lineage origin** — the `generatedAt` of the + * snapshot it supersedes — rather than stamping today. + * + * That matters because [scoreOutstanding] measures backfill from this date. + * Regenerating it moved the goalposts: a "Not yet" on the 29th followed by + * *"it actually started on the 28th"* logged on the 30th tripped the backfill + * guard, and the forecast the user was actually shown was then deleted here, + * unscored. The cycle contributed no sample at all — the app failing to learn + * from precisely the cycle it got wrong. + * + * Simpler predicates were tried and rejected: *"score if this is the new + * latest start"* cannot be told apart from onboarding's ascending backfill at + * a fixed clock, and breaks `backfilled history does not fabricate accuracy + * figures`. The lineage origin passes both — an onboarding lineage begins at + * the onboarding moment, so every historical entry still trips the guard, + * while a real cycle's lineage begins at the confirm that opened it. + * + * The accepted trade-off: a retro-logged start scores the *final* revision of + * the standing forecast, so where the user's own "Not yet" contradicted what + * they later logged, the error is inflated by at most that contradiction. + * Bounded, and honest about what was on screen. The alternative was silence. + * + * `null` origin — no standing snapshot — happens exactly when the last one + * was just scored, when none existed, or when a lineage died with its + * history. All three are genuine restarts, and take today. */ private suspend fun snapshotForecast(basedOnPeriodId: Long?) { val starts = periodDao.confirmedStartDates() // LocalDate.EPOCH is API 34. ofEpochDay(0) is the same date and is API 26. val notYet = notYetDao.since(LocalDate.ofEpochDay(0)).map { it.toDomain() } val errors = predictionDao.recentAbsoluteErrors() + + // Read before the delete below, or the lineage is lost with the row. + val lineageBegan = predictionDao.unscored().firstOrNull()?.generatedAt + + // Retire the standing forecast BEFORE consulting the engine, not after. + // + // The engine returns null when there is no history left to forecast from + // — the user just deleted their last period. Returning early without + // this left a forecast about a history that no longer exists waiting to + // be scored, and the next confirmed start, belonging to an unrelated + // fresh history, scored it. A lineage has to die with its history. + predictionDao.deleteUnscored() + val prediction = engine.predict( PredictionInput(starts, today(), notYet, errors), ) ?: return - predictionDao.deleteUnscored() - predictionDao.insert(prediction.toEntity(clock.instant(), basedOnPeriodId)) + predictionDao.insert( + prediction.toEntity(lineageBegan ?: clock.instant(), basedOnPeriodId), + ) } } diff --git a/core/data/src/test/kotlin/dev/privacyllc/period/core/data/CycleRepositoryTest.kt b/core/data/src/test/kotlin/dev/privacyllc/period/core/data/CycleRepositoryTest.kt index 0416936..ffd6dbe 100644 --- a/core/data/src/test/kotlin/dev/privacyllc/period/core/data/CycleRepositoryTest.kt +++ b/core/data/src/test/kotlin/dev/privacyllc/period/core/data/CycleRepositoryTest.kt @@ -285,6 +285,116 @@ class CycleRepositoryTest { assertEquals(1, later.accuracy.first().scoredCount) } + // ----------------------------------------------------------------------- + // The learning loop: a lineage of forecasts, and scores that follow their + // ground truth. Every test below pins a defect found by tracing the loop + // end to end — each one silently cost the app a sample or fed it a false + // one, which is the difference between learning and merely measuring. + // ----------------------------------------------------------------------- + + @Test fun `deleting the last period retires the standing forecast`() = runTest { + val id = (repo.confirmPeriodStart(LocalDate.of(2026, 8, 1)) as PeriodWriteResult.Added).id + assertEquals(1, db.predictionRecordDao().unscored().size) + + // With no history the engine returns null. The forecast it was about is + // gone, so the forecast must go too — it used to survive the early + // return and wait around to be scored against a stranger. + repo.deletePeriod(id) + assertTrue(db.predictionRecordDao().unscored().isEmpty()) + + val later = repoAt(LocalDate.of(2026, 9, 10)) + later.confirmPeriodStart(LocalDate.of(2026, 9, 10)) + assertEquals(0, later.accuracy.first().scoredCount) + } + + @Test fun `a period logged retroactively still scores the forecast the user was shown`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") // lineage begins 2026-08-18 + + // "Not yet" on the 29th, then the period turns out to have started on + // the 28th and is logged on the 30th. The revision used to restamp the + // forecast's date, trip the backfill guard, and delete the evidence: + // the app learned nothing from the cycle it had just got wrong. + repoAt(LocalDate.of(2026, 8, 29)).recordNotYet(LocalDate.of(2026, 8, 29)) + + val morningAfter = repoAt(LocalDate.of(2026, 8, 30)) + morningAfter.confirmPeriodStart(LocalDate.of(2026, 8, 28)) + + assertEquals(1, morningAfter.accuracy.first().scoredCount) + assertEquals(1, db.predictionRecordDao().unscored().size) + } + + @Test fun `a not-yet dated after a retroactively logged start does not haunt the next cycle`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") + repoAt(LocalDate.of(2026, 8, 29)).recordNotYet(LocalDate.of(2026, 8, 29)) + + // The confirm resolves the question every observation was censoring, + // including the one dated after the start itself — which a date-bounded + // clear cannot reach, leaving it to penalise the next cycle's forecast. + repoAt(LocalDate.of(2026, 8, 30)).confirmPeriodStart(LocalDate.of(2026, 8, 28)) + + assertTrue(repo.notYetObservations.first().isEmpty()) + } + + @Test fun `a deep backfill does not clear the observations censoring the standing question`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") + val atNotYet = repoAt(LocalDate.of(2026, 8, 27)) + atNotYet.recordNotYet(LocalDate.of(2026, 8, 27)) + + // Remembering a period from May resolves nothing about when the next one + // starts, so the live observation stays. The pin against overshooting + // the fix above into "any confirm clears everything". + atNotYet.confirmPeriodStart(LocalDate.of(2026, 5, 3), PeriodRecordSource.HISTORICAL_ENTRY) + + assertEquals(LocalDate.of(2026, 8, 27), repo.notYetObservations.first().single().date) + } + + @Test fun `editing a period re-scores the forecast that was scored against it`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") // forecast: 2026-08-27 + val late = repoAt(LocalDate.of(2026, 8, 29)) + val id = (late.confirmPeriodStart(LocalDate.of(2026, 8, 29)) as PeriodWriteResult.Added).id + assertEquals(2, late.accuracy.first().lastErrorDays) + + // Correcting the date corrects the measurement taken against it. The + // stale error used to keep feeding both §16's figures and the engine's + // own window — the app learning from a period the user had rewritten. + late.editPeriod(id, LocalDate.of(2026, 8, 26), null) + + assertEquals(1, late.accuracy.first().lastErrorDays) + assertEquals(1, late.accuracy.first().scoredCount) + val scored = db.predictionRecordDao().observeScored().first().single() + assertEquals(LocalDate.of(2026, 8, 26), scored.actualStartDate) + // What was predicted is never rewritten, which is what keeps a re-score + // a recomputation rather than curation. + assertEquals(LocalDate.of(2026, 8, 27), scored.predictedStartDate) + } + + @Test fun `editing a period to before its forecast existed retracts the score`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") // lineage begins 2026-08-18 + val late = repoAt(LocalDate.of(2026, 8, 29)) + val id = (late.confirmPeriodStart(LocalDate.of(2026, 8, 29)) as PeriodWriteResult.Added).id + assertEquals(1, late.accuracy.first().scoredCount) + + // Moved to before the forecast was ever made: the same rule that refuses + // to score backfill refuses to keep this one. An edit must not be able + // to smuggle in a measurement the guard would have turned away. + late.editPeriod(id, LocalDate.of(2026, 8, 10), null) + + assertEquals(0, late.accuracy.first().scoredCount) + } + + @Test fun `deleting a period retracts the score recorded against it`() = runTest { + seed("2026-06-01", "2026-06-30", "2026-07-29") + val late = repoAt(LocalDate.of(2026, 8, 29)) + val id = (late.confirmPeriodStart(LocalDate.of(2026, 8, 29)) as PeriodWriteResult.Added).id + assertEquals(1, late.accuracy.first().scoredCount) + + late.deletePeriod(id) + + assertEquals(0, late.accuracy.first().scoredCount) + // And the engine stops being told about it, not just the accuracy card. + assertTrue(db.predictionRecordDao().recentAbsoluteErrors().isEmpty()) + } + @Test fun `only one forecast stands at a time`() = runTest { seed("2026-06-01", "2026-06-30", "2026-07-29") repo.recordNotYet(LocalDate.of(2026, 8, 18)) diff --git a/core/database/src/main/kotlin/dev/privacyllc/period/core/database/dao/Daos.kt b/core/database/src/main/kotlin/dev/privacyllc/period/core/database/dao/Daos.kt index 4df0770..a012cdc 100644 --- a/core/database/src/main/kotlin/dev/privacyllc/period/core/database/dao/Daos.kt +++ b/core/database/src/main/kotlin/dev/privacyllc/period/core/database/dao/Daos.kt @@ -21,6 +21,11 @@ import java.time.LocalDate * Note there is no `deleteAll`-by-convenience anywhere except the explicit * `deleteEverything` on the database itself: Delete My Data is an irreversible * operation the user confirms, not something a DAO offers casually. + * + * One documented exception: [NotYetObservationDao.deleteAll]. Every observation + * censors the same single question, so a confirmed start resolves all of them at + * once by definition — see the comment there for why a date-bounded delete could + * not express that. */ @Dao interface PeriodRecordDao { @@ -122,10 +127,35 @@ interface PredictionRecordDao { @Insert(onConflict = OnConflictStrategy.ABORT) suspend fun insert(record: PredictionRecordEntity): Long + /** + * The snapshots whose recorded outcome is a period starting on [actual]. + * + * Matched on the outcome rather than `basedOnLastConfirmedPeriodId`, which + * is null on the "not yet" and delete paths and cannot identify anything. + */ + @Query("SELECT * FROM prediction_records WHERE actualStartDate = :actual") + suspend fun scoredAgainst(actual: LocalDate): List + + /** + * Retraction — the one way a scored snapshot dies short of Delete My Data. + * + * A measurement whose ground truth was edited into impossibility or deleted + * outright is not a measurement. Leaving it would keep feeding both the + * engine's error window (§12 step 5) and §16's figures an error about a + * period that, per the user, never happened that way. + */ + @Query("DELETE FROM prediction_records WHERE id = :id") + suspend fun deleteById(id: Long) + /** * The ONLY permitted mutation of a snapshot: recording what actually * happened. Deliberately not an `@Update` of the whole row, so a caller * cannot rewrite what was predicted after learning the answer. + * + * Correcting an outcome — the user edits the period this was scored against + * — runs through this same gate, for the same reason. `predictedStartDate` + * stays untouchable, which is what keeps a re-score a recomputation rather + * than curation: it can worsen the figure as easily as improve it. */ @Query("UPDATE prediction_records SET actualStartDate = :actual, absoluteErrorDays = :errorDays WHERE id = :id") suspend fun score(id: Long, actual: LocalDate, errorDays: Int) @@ -146,4 +176,19 @@ interface NotYetObservationDao { /** Cleared when a period is confirmed: they censored a forecast that is now resolved. */ @Query("DELETE FROM not_yet_observations WHERE date < :before") suspend fun deleteBefore(before: LocalDate) + + /** + * The deliberate exception to this file's no-`deleteAll` rule. + * + * Every stored observation censors exactly one question — *when does the + * period after the latest confirmed start begin?* — so a confirm that + * resolves that question moots all of them at once, whatever their dates. + * + * [deleteBefore] cannot express that. A period logged retroactively leaves + * every observation dated after it untouched, and those then carry a + * resolved cycle's "not yet"s into the next forecast: a confidence penalty + * and a window floor for a question that is already answered. + */ + @Query("DELETE FROM not_yet_observations") + suspend fun deleteAll() } diff --git a/core/database/src/main/kotlin/dev/privacyllc/period/core/database/entity/Entities.kt b/core/database/src/main/kotlin/dev/privacyllc/period/core/database/entity/Entities.kt index bf15e35..cfbe4f7 100644 --- a/core/database/src/main/kotlin/dev/privacyllc/period/core/database/entity/Entities.kt +++ b/core/database/src/main/kotlin/dev/privacyllc/period/core/database/entity/Entities.kt @@ -61,6 +61,14 @@ data class SpottingRecordEntity( ) data class PredictionRecordEntity( @PrimaryKey(autoGenerate = true) val id: Long = 0, + /** + * When this line of forecasting began — **not** when this row was inserted. + * + * Carried across every revision of the standing forecast (a "Not yet", an + * edit, a delete), so a revision does not make the question look newer than + * it is. `CycleRepository.scoreOutstanding` measures backfill from this date, + * and `CycleRepository.snapshotForecast` explains why. + */ val generatedAt: Instant, val basedOnLastConfirmedPeriodId: Long?, val predictedStartDate: LocalDate, diff --git a/docs/architecture/README.md b/docs/architecture/README.md index a16a33e..3db2848 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -176,6 +176,41 @@ ever shown. This was a real defect, caught by a test expecting one scored prediction and finding three, and it is pinned by `backfilled history does not fabricate accuracy figures`. +**`generatedAt` is when the forecast's *lineage* began, not when the row was +written.** A "Not yet", an edit and a delete each revise the answer to one +standing question — *when does the period after the latest confirmed start +begin?* — so the replacement snapshot inherits the superseded one's date rather +than stamping today. Restamping it moved the goalposts of the backfill rule +above: a "Not yet" on the 29th, then a period logged on the 30th as having +started on the 28th, tripped the guard and the forecast the user was actually +shown was deleted unscored. The app learned nothing from precisely the cycle it +got wrong. Pinned by `a period logged retroactively still scores the forecast the +user was shown`. For the same reason the standing snapshot is retired *before* +the engine is consulted, so a lineage dies with its history rather than waiting +to be scored against an unrelated one — `deleting the last period retires the +standing forecast`. + +**A score follows the period it is a fact about.** Editing a confirmed start +re-scores every snapshot recorded against it (`predictedStartDate` stays +immutable, so a correction worsens the figure as readily as it improves one), +unless the corrected start falls before the lineage began — the same rule that +refuses to score backfill refuses to keep that one, so an edit cannot smuggle in +a measurement the guard would have turned away. Deleting the period retracts its +score outright. Without this, §16's figures and the engine's own error window +kept learning from a period the user had rewritten or withdrawn. Pinned by +`editing a period re-scores the forecast that was scored against it`, +`editing a period to before its forecast existed retracts the score` and +`deleting a period retracts the score recorded against it`. + +**A resolving confirm clears every "not yet", not just the older ones.** They all +censor the same question, so the newest confirmed start moots all of them at +once; a date-bounded clear left observations dated after a retroactively logged +start alive to penalise the next cycle's confidence for a question already +answered. A backfill into the middle of history still clears only what precedes +it, since it resolves nothing about the standing question — `a not-yet dated +after a retroactively logged start does not haunt the next cycle` and `a deep +backfill does not clear the observations censoring the standing question`. + ### The boundary that is not negotiable > The advertising subsystem must never receive menstrual dates, cycle length,