diff --git a/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepositoryTest.kt b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepositoryTest.kt new file mode 100644 index 000000000..61e88e0ee --- /dev/null +++ b/shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepositoryTest.kt @@ -0,0 +1,31 @@ +package com.devil.phoenixproject.data.repository + +import com.devil.phoenixproject.domain.onerepmax.VelocityOneRepMaxResult +import com.devil.phoenixproject.testutil.createTestDatabase +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlinx.coroutines.test.runTest + +class VelocityOneRepMaxRepositoryTest { + + @Test fun `latest passing skips newer floor row and returns older usable estimate`() = runTest { + val repository = SqlDelightVelocityOneRepMaxRepository(createTestDatabase()) + + repository.insert(result(100f), exerciseId = "row", computedAt = 1_000L, profileId = "default") + repository.insert(result(1f), exerciseId = "row", computedAt = 2_000L, profileId = "default") + + val latest = repository.getLatestPassing("row", "default") + + assertNotNull(latest) + assertEquals(100f, latest.estimatedPerCableKg) + } + + private fun result(estimatedPerCableKg: Float) = VelocityOneRepMaxResult( + estimatedPerCableKg = estimatedPerCableKg, + mvtUsedMs = 0.5f, + r2 = 0.9f, + distinctLoads = 3, + passedQualityGate = true, + ) +} diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepository.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepository.kt index 9074f757b..6b29b0485 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepository.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/VelocityOneRepMaxRepository.kt @@ -3,6 +3,7 @@ package com.devil.phoenixproject.data.repository import app.cash.sqldelight.coroutines.asFlow import app.cash.sqldelight.coroutines.mapToList import com.devil.phoenixproject.database.VitruvianDatabase +import com.devil.phoenixproject.domain.onerepmax.VelocityOneRepMaxEstimator import com.devil.phoenixproject.domain.onerepmax.VelocityOneRepMaxResult import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO @@ -61,12 +62,27 @@ class SqlDelightVelocityOneRepMaxRepository(private val db: VitruvianDatabase) : override suspend fun getLatestPassing(exerciseId: String, profileId: String): VelocityOneRepMaxEntity? = withContext(Dispatchers.IO) { - queries.selectLatestPassingVelocityOneRepMax(exerciseId, profileId, ::map).executeAsOneOrNull() + // Issue #644: ignore floor-clamped rows (AssessmentEngine's 1.0 kg hardware floor) + // when selecting the latest passing baseline. Affected users have a poisoned row + // from a previous build that would short-circuit stored-1RM / max-weight PR + // fallback. Filter here so editor preview and workout-start resolution agree. + queries.selectLatestPassingVelocityOneRepMax( + exerciseId, + profileId, + VelocityOneRepMaxEstimator.MIN_USABLE_ESTIMATE_KG.toDouble(), + ::map, + ).executeAsOneOrNull() } override suspend fun getAllPassing(profileId: String): List = withContext(Dispatchers.IO) { - queries.selectAllPassingVelocityOneRepMaxByProfile(profileId, ::map).executeAsList() + // Issue #644: same floor filter for callers (e.g., PR surfaces) that enumerate + // all passing estimates. + queries.selectAllPassingVelocityOneRepMaxByProfile( + profileId, + VelocityOneRepMaxEstimator.MIN_USABLE_ESTIMATE_KG.toDouble(), + ::map, + ).executeAsList() } override fun getHistory(exerciseId: String, profileId: String): Flow> = diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimator.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimator.kt index e5e356ccc..9d4eea2f1 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimator.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimator.kt @@ -47,7 +47,13 @@ class VelocityOneRepMaxEstimator(private val assessmentEngine: AssessmentEngine) val config = AssessmentConfig(minSets = MIN_DISTINCT_LOADS, oneRmVelocityMs = mvtMs) val assessment = assessmentEngine.estimateOneRepMax(lvPoints, config) ?: return null - val passed = distinctLoads >= MIN_DISTINCT_LOADS && assessment.r2 >= R2_PASS_THRESHOLD + // Issue #644: AssessmentEngine clamps the extrapolated load to the 1.0 kg hardware + // floor. A result at the floor means the regression couldn't reach the 1RM velocity + // — that's a diagnostic value, not a usable per-cable 1RM. Mark it unpassing so the + // resolver falls through to stored-1RM / max-weight PR fallback. + val passed = assessment.estimatedOneRepMaxKg > MIN_USABLE_ESTIMATE_KG && + distinctLoads >= MIN_DISTINCT_LOADS && + assessment.r2 >= R2_PASS_THRESHOLD return VelocityOneRepMaxResult( estimatedPerCableKg = assessment.estimatedOneRepMaxKg, mvtUsedMs = mvtMs, @@ -63,5 +69,20 @@ class VelocityOneRepMaxEstimator(private val assessmentEngine: AssessmentEngine) const val R2_PASS_THRESHOLD = 0.8f const val MIN_DISTINCT_LOADS = 3 const val LOAD_BUCKET_KG = 0.5f + + /** + * Minimum per-cable 1RM estimate that may be treated as a real baseline. Values at or + * below the AssessmentEngine hardware floor (1.0 kg) are diagnostic, not actionable — + * the regression couldn't reach the 1RM velocity, so 80% of that rounds to 0 and + * collapses the routine to a meaningless weight. Issue #644. + */ + const val MIN_USABLE_ESTIMATE_KG = 1.0f + + /** + * Predicate: true when [estimatedPerCableKg] is a usable velocity-1RM baseline. Shared + * by producer, repository, resolver, and editor so they all reject floor-clamped rows. + */ + fun isUsableEstimate(estimatedPerCableKg: Float): Boolean = + estimatedPerCableKg > MIN_USABLE_ESTIMATE_KG } } diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/usecase/ComputeVelocityOneRepMaxUseCase.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/usecase/ComputeVelocityOneRepMaxUseCase.kt index 5a0881f82..6879da21c 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/usecase/ComputeVelocityOneRepMaxUseCase.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/usecase/ComputeVelocityOneRepMaxUseCase.kt @@ -29,6 +29,12 @@ class ComputeVelocityOneRepMaxUseCase( personalSampleCount = personal?.second ?: 0, ) val result = estimator.estimate(points, mvt) ?: return null + // Issue #644: never persist a passing velocity-1RM row that sits at the 1.0 kg + // hardware floor — the regression didn't actually reach the 1RM velocity, and + // persisting it would let the resolver short-circuit stored-1RM / max-weight PR + // fallback. Returning null here keeps the call sites' null-handling intact and + // mirrors what a row with r2 0 } + // Issue #644: reject the AssessmentEngine 1.0 kg hardware floor. A row at the + // floor is a diagnostic, not a usable baseline — fall through to stored-1RM / + // max-weight PR fallback so 80% scaling doesn't collapse to 2.2 lb/cable. + // Belt-and-suspenders: SqlDelightVelocityOneRepMaxRepository already filters, + // but a different repo impl plugged in here would re-introduce the bug. + ?.takeIf { VelocityOneRepMaxEstimator.isUsableEstimate(it) } ?.let { estimate -> return RoutineScalingBaseline( weightPerCableKg = estimate, 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 94bf9366a..7c1c160a4 100644 --- a/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq +++ b/shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq @@ -483,12 +483,14 @@ selectLatestPassingVelocityOneRepMax: SELECT * FROM VelocityOneRepMaxEstimate WHERE exerciseId = :exerciseId AND profile_id = :profileId AND passedQualityGate = 1 AND deletedAt IS NULL +AND estimatedPerCableKg > :minUsableEstimateKg ORDER BY computedAt DESC, id DESC LIMIT 1; selectAllPassingVelocityOneRepMaxByProfile: SELECT * FROM VelocityOneRepMaxEstimate WHERE profile_id = :profileId AND passedQualityGate = 1 AND deletedAt IS NULL +AND estimatedPerCableKg > :minUsableEstimateKg ORDER BY exerciseId ASC, computedAt ASC; -- Issue #517 Phase 5 T1: enumerate exercises that have at least one MCV-bearing set. diff --git a/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimatorTest.kt b/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimatorTest.kt index 909fb94b8..80bb649a5 100644 --- a/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimatorTest.kt +++ b/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/onerepmax/VelocityOneRepMaxEstimatorTest.kt @@ -2,6 +2,8 @@ package com.devil.phoenixproject.domain.onerepmax import com.devil.phoenixproject.domain.assessment.AssessmentEngine import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -52,4 +54,40 @@ class VelocityOneRepMaxEstimatorTest { // estimator trusts caller to pass working points; verify it still needs 3 distinct loads assertNull(estimator.estimate(listOf(point(50f, 700f)), mvtMs = 0.20f)) } + + // Issue #644: a velocity-1RM estimate that hits AssessmentEngine's 1.0 kg hardware floor + // means the regression didn't reach the 1RM velocity — diagnostic, not a usable baseline. + // The estimator must mark it unpassing so the resolver falls through to stored-1RM / PR + // fallback and the producer (ComputeVelocityOneRepMaxUseCase) never persists it. + + @Test fun `estimate that lands on the 1 kg hardware floor is not passing`() { + // Points (1kg@0.5, 2kg@0.3, 3kg@0.1 m/s) → OLS slope -0.20, intercept 0.70. + // At mvtMs=0.50: load = (0.50 - 0.70) / -0.20 = 1.0 kg → hardware floor. + val result = estimator.estimate( + points = listOf( + point(1f, 500f), + point(2f, 300f), + point(3f, 100f), + ), + mvtMs = 0.50f, + ) + assertNotNull(result, "estimator should still return a result for visibility") + assertTrue(result.estimatedPerCableKg <= VelocityOneRepMaxEstimator.MIN_USABLE_ESTIMATE_KG, + "expected estimate at/below floor, got ${result.estimatedPerCableKg}") + assertFalse(result.passedQualityGate, + "a floor-clamped estimate must not be marked passing (Issue #644)") + } + + @Test fun `isUsableEstimate predicate matches the documented contract`() { + // The shared predicate is the single source of truth — verify the boundary. + assertFalse(VelocityOneRepMaxEstimator.isUsableEstimate(0f)) + assertFalse(VelocityOneRepMaxEstimator.isUsableEstimate(1.0f)) + assertTrue(VelocityOneRepMaxEstimator.isUsableEstimate(1.5f)) + assertTrue(VelocityOneRepMaxEstimator.isUsableEstimate(100f)) + } + + @Test fun `MIN_USABLE_ESTIMATE_KG equals 1 kg`() { + // Document the floor explicitly so a future refactor doesn't drift the contract. + assertEquals(1.0f, VelocityOneRepMaxEstimator.MIN_USABLE_ESTIMATE_KG) + } } diff --git a/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/usecase/ResolveRoutineWeightsUseCaseTest.kt b/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/usecase/ResolveRoutineWeightsUseCaseTest.kt index 739c964dc..64444a02f 100644 --- a/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/usecase/ResolveRoutineWeightsUseCaseTest.kt +++ b/shared/src/commonTest/kotlin/com/devil/phoenixproject/domain/usecase/ResolveRoutineWeightsUseCaseTest.kt @@ -695,6 +695,135 @@ class ResolveRoutineWeightsUseCaseTest { assertNull(result.fallbackReason) } + // Issue #644: a velocity-1RM row at the 1.0 kg hardware floor must not be selected as the + // baseline — it would short-circuit stored-1RM / max-weight PR fallback and collapse 80% + // scaling to 2.2 lb/cable. The repo now filters such rows out; verify the resolver agrees + // and falls through to the PR / stored-1RM baseline instead. + + @Test + fun `ESTIMATED_1RM ignores floor-clamped velocity row and falls back to stored 1RM`() = runTest { + // Given: a poisoned velocity row at the 1.0 kg floor plus a sane stored 1RM + velocityRepository.latestPassing = VelocityOneRepMaxEntity( + id = 1L, + exerciseId = "bench-press", + estimatedPerCableKg = 1.0f, // AssessmentEngine floor clamp + mvtUsedMs = 0.3f, + r2 = 0.98f, + distinctLoads = 4, + passedQualityGate = true, // legacy row from a previous build + computedAt = 1000L, + profileId = "default", + ) + exerciseRepository.addExercise(testExercise.copy(oneRepMaxKg = 120f)) + + val routineExercise = RoutineExercise( + id = "routine-ex-1rm-floor-stored", + exercise = testExercise, + orderIndex = 0, + weightPerCableKg = 30f, + usePercentOfPR = true, + weightPercentOfPR = 80, + scalingBasis = ScalingBasis.ESTIMATED_1RM, + programMode = ProgramMode.OldSchool, + ) + + val result = useCase(routineExercise) + + // 80% of stored 120 = 96 kg — not 1.0 kg / 2.2 lb + assertEquals(96f, result.baseWeight) + assertEquals(120f, result.usedPR) + assertEquals(80, result.percentOfPR) + assertTrue(result.isFromPR) + assertNull(result.fallbackReason) + } + + @Test + fun `ESTIMATED_1RM ignores floor-clamped velocity row and falls back to max-weight PR`() = runTest { + // Given: floor-clamped velocity row + no stored 1RM + a max-weight PR + velocityRepository.latestPassing = VelocityOneRepMaxEntity( + id = 2L, + exerciseId = "bench-press", + estimatedPerCableKg = 1.0f, + mvtUsedMs = 0.3f, + r2 = 0.95f, + distinctLoads = 4, + passedQualityGate = true, + computedAt = 1000L, + profileId = "default", + ) + prRepository.addRecord( + PersonalRecord( + id = 1, + exerciseId = "bench-press", + exerciseName = "Bench Press", + weightPerCableKg = 60f, + reps = 5, + oneRepMax = 70f, + timestamp = 1000L, + workoutMode = "Old School", + prType = PRType.MAX_WEIGHT, + volume = 300f, + ), + ) + + val routineExercise = RoutineExercise( + id = "routine-ex-1rm-floor-pr", + exercise = testExercise, + orderIndex = 0, + weightPerCableKg = 30f, + usePercentOfPR = true, + weightPercentOfPR = 80, + scalingBasis = ScalingBasis.ESTIMATED_1RM, + programMode = ProgramMode.OldSchool, + ) + + val result = useCase(routineExercise) + + // 80% of 60 (max-weight PR) = 48 kg — not 1.0 kg + assertEquals(48f, result.baseWeight) + assertEquals(60f, result.usedPR) + assertEquals(80, result.percentOfPR) + assertTrue(result.isFromPR) + assertNull(result.fallbackReason) + } + + @Test + fun `ESTIMATED_1RM still trusts a sane velocity row above the floor`() = runTest { + // Sanity: the new filter must not strip legitimate estimates. + velocityRepository.latestPassing = VelocityOneRepMaxEntity( + id = 3L, + exerciseId = "bench-press", + estimatedPerCableKg = 100f, + mvtUsedMs = 0.3f, + r2 = 0.98f, + distinctLoads = 4, + passedQualityGate = true, + computedAt = 1000L, + profileId = "default", + ) + exerciseRepository.addExercise(testExercise.copy(oneRepMaxKg = 120f)) + + val routineExercise = RoutineExercise( + id = "routine-ex-1rm-sane", + exercise = testExercise, + orderIndex = 0, + weightPerCableKg = 30f, + usePercentOfPR = true, + weightPercentOfPR = 80, + scalingBasis = ScalingBasis.ESTIMATED_1RM, + programMode = ProgramMode.OldSchool, + ) + + val result = useCase(routineExercise) + + // 80% of 100 = 80 kg + assertEquals(80f, result.baseWeight) + assertEquals(100f, result.usedPR) + assertEquals(80, result.percentOfPR) + assertTrue(result.isFromPR) + assertNull(result.fallbackReason) + } + @Test fun `max weight basis falls back to same profile cross mode PR when selected mode has none`() = runTest { prRepository.addRecord(