diff --git a/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepositoryTest.kt b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepositoryTest.kt index 8a598c6f..5e4d703e 100644 --- a/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepositoryTest.kt +++ b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepositoryTest.kt @@ -283,6 +283,37 @@ class SqlDelightPersonalRecordRepositoryTest { assertEquals(70f, restoredPr.weightPerCableKg) } + @Test + fun `a lower valid PR replaces a deleted outlier with its stable sync uuid`() = runTest { + repository.updatePRsIfBetter( + exerciseId = "bench", + weightPRWeightPerCableKg = 100f, + volumePRWeightPerCableKg = 100f, + reps = 5, + workoutMode = "Old School", + timestamp = 2_000L, + profileId = "default", + ).getOrThrow() + val deletedPr = assertNotNull(repository.getWeightPR("bench", "Old School", "default")) + val stableUuid = assertNotNull(deletedPr.uuid) + repository.deletePR(deletedPr.id, "default") + + val result = repository.updatePRsIfBetter( + exerciseId = "bench", + weightPRWeightPerCableKg = 90f, + volumePRWeightPerCableKg = 90f, + reps = 5, + workoutMode = "Old School", + timestamp = 3_000L, + profileId = "default", + ).getOrThrow() + + val replacementPr = assertNotNull(repository.getWeightPR("bench", "Old School", "default")) + assertTrue(result.contains(PRType.MAX_WEIGHT)) + assertEquals(stableUuid, replacementPr.uuid) + assertEquals(90f, replacementPr.weightPerCableKg) + } + private fun insertExercise(id: String, name: String) { database.vitruvianDatabaseQueries.insertExercise( id = id, diff --git a/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightSyncRepositoryTest.kt b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightSyncRepositoryTest.kt index 9796797e..a25a7cec 100644 --- a/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightSyncRepositoryTest.kt +++ b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightSyncRepositoryTest.kt @@ -172,30 +172,32 @@ class SqlDelightSyncRepositoryTest { @Test fun `mergePersonalRecords keeps a newer tombstone and rejects stale active replay`() = runTest { val prId = "12345678-1234-4abc-8def-1234567890cc" - fun syncDto(updatedAt: Long, deletedAt: Long?) = PersonalRecordSyncDto( + fun syncDto(updatedAt: Long, deletedAt: Long?, weight: Float = 85f) = PersonalRecordSyncDto( clientId = prId, serverId = prId, exerciseId = "deadlift", exerciseName = "Deadlift", - weight = 85f, + weight = weight, reps = 5, - oneRepMax = 99.17f, + oneRepMax = weight * 1.1667f, achievedAt = 1_700_000_000_000L, workoutMode = "Old School", prType = PRType.MAX_WEIGHT.name, - volume = 425f, + volume = weight * 5, createdAt = 1_700_000_000_000L, updatedAt = updatedAt, deletedAt = deletedAt, ) repository.mergePersonalRecords(listOf(syncDto(100L, null)), "active-profile") - repository.mergePersonalRecords(listOf(syncDto(200L, 200L)), "active-profile") + repository.mergePersonalRecords(listOf(syncDto(200L, 200L, weight = 0f)), "active-profile") repository.mergePersonalRecords(listOf(syncDto(150L, null)), "active-profile") val row = database.vitruvianDatabaseQueries.selectAllRecordsSync().executeAsOne() assertEquals(200L, row.updatedAt) assertEquals(200L, row.deletedAt) + assertEquals(85.0, row.weight) + assertEquals(425.0, row.volume) } @Test diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepository.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepository.kt index f0593a86..1d1ff584 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepository.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepository.kt @@ -296,8 +296,9 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe // Issue #319: Log the profile context being used Logger.d { "PR_SAVE: Checking for exercise=$exerciseId, mode=$canonicalWorkoutMode, phase=$phaseName, profile=$effectiveProfileId" } - // Check weight PR for this phase - val currentWeightPR = queries.selectPRIncludingDeleted( + // Keep a tombstone's UUID for sync identity, but do not let a deleted + // outlier remain the active comparison baseline for future PRs. + val storedWeightPR = queries.selectPRIncludingDeleted( exerciseId, canonicalWorkoutMode, PRType.MAX_WEIGHT.name, @@ -306,6 +307,7 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe mapper = ::mapToPR, ) .executeAsOneOrNull() + val currentWeightPR = storedWeightPR?.takeIf { it.deletedAt == null } // Issue #319: Detect profile mismatch if a PR exists with a different profile_id if (currentWeightPR != null && currentWeightPR.profileId != effectiveProfileId) { @@ -321,8 +323,8 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe "new=${weightPRWeightPerCableKg}kg vs current=${currentWeightPR?.weightPerCableKg ?: "NONE"} → ${if (isNewWeightPR) "NEW PR" else "no change"}" } - // Check volume PR for this phase - val currentVolumePR = queries.selectPRIncludingDeleted( + // Keep the deleted volume snapshot only for UUID reuse, not comparison. + val storedVolumePR = queries.selectPRIncludingDeleted( exerciseId, canonicalWorkoutMode, PRType.MAX_VOLUME.name, @@ -331,6 +333,7 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe mapper = ::mapToPR, ) .executeAsOneOrNull() + val currentVolumePR = storedVolumePR?.takeIf { it.deletedAt == null } // Issue #319: Detect profile mismatch for volume PR too if (currentVolumePR != null && currentVolumePR.profileId != effectiveProfileId) { @@ -364,7 +367,9 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe phase = phaseName, profile_id = effectiveProfileId, cable_count = cableCount?.toLong(), - uuid = currentWeightPR?.uuid ?: generateUUID(), + // A deleted snapshot is no longer the active comparison + // baseline, but its UUID is retained until sync converges. + uuid = storedWeightPR?.uuid ?: generateUUID(), ) brokenPRs.add(PRType.MAX_WEIGHT) } @@ -384,7 +389,7 @@ class SqlDelightPersonalRecordRepository(private val db: VitruvianDatabase) : Pe phase = phaseName, profile_id = effectiveProfileId, cable_count = cableCount?.toLong(), - uuid = currentVolumePR?.uuid ?: generateUUID(), + uuid = storedVolumePR?.uuid ?: generateUUID(), ) brokenPRs.add(PRType.MAX_VOLUME) } diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt index 5ebdab2e..b581029e 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt @@ -545,9 +545,8 @@ fun AnalyticsScreen( onDeletePersonalRecord = { pr -> personalRecordRepository.deletePR(pr.id, pr.profileId) }, - onDeletePersonalRecordError = { error -> - exportMessage = error.message?.takeIf { it.isNotBlank() } - ?: deletePersonalRecordFailed + onDeletePersonalRecordError = { _ -> + exportMessage = deletePersonalRecordFailed }, modifier = Modifier.fillMaxSize(), ) diff --git a/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq b/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq index a82a1fb7..9c4d689e 100644 --- a/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq +++ b/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq @@ -1147,13 +1147,16 @@ applyPulledPersonalRecordTombstoneState: UPDATE PersonalRecord SET deletedAt = :deletedAt, updatedAt = :updatedAt, - exerciseName = :exerciseName, - weight = :weight, - reps = :reps, - oneRepMax = :oneRepMax, - achievedAt = :achievedAt, - volume = :volume, - cable_count = :cableCount + -- Portal tombstone DTOs intentionally contain only identity and deletion + -- state. Preserve the last active metrics for a tombstone, but replace the + -- full snapshot when a newer active record legitimately restores it. + exerciseName = CASE WHEN :deletedAt IS NULL THEN :exerciseName ELSE exerciseName END, + weight = CASE WHEN :deletedAt IS NULL THEN :weight ELSE weight END, + reps = CASE WHEN :deletedAt IS NULL THEN :reps ELSE reps END, + oneRepMax = CASE WHEN :deletedAt IS NULL THEN :oneRepMax ELSE oneRepMax END, + achievedAt = CASE WHEN :deletedAt IS NULL THEN :achievedAt ELSE achievedAt END, + volume = CASE WHEN :deletedAt IS NULL THEN :volume ELSE volume END, + cable_count = CASE WHEN :deletedAt IS NULL THEN :cableCount ELSE cable_count END WHERE uuid = :uuid AND profile_id = :profileId AND ( diff --git a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsPersonalRecordDeleteReviewTest.kt b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsPersonalRecordDeleteReviewTest.kt index a3dc3a13..a29bb623 100644 --- a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsPersonalRecordDeleteReviewTest.kt +++ b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsPersonalRecordDeleteReviewTest.kt @@ -3,6 +3,7 @@ package com.devil.phoenixproject.presentation.screen import com.devil.phoenixproject.testutil.readProjectFile import kotlin.test.Test import kotlin.test.assertContains +import kotlin.test.assertFalse import kotlin.test.assertNotNull class AnalyticsPersonalRecordDeleteReviewTest { @@ -24,6 +25,7 @@ class AnalyticsPersonalRecordDeleteReviewTest { pendingDelete = null }""", ) - assertContains(analytics, "exportMessage = error.message?.takeIf { it.isNotBlank() }") + assertContains(analytics, "exportMessage = deletePersonalRecordFailed") + assertFalse(analytics.contains("exportMessage = error.message?.takeIf { it.isNotBlank() }")) } }