From 8dc3fcf357add33aa821c2132eb3285b2f306f6e Mon Sep 17 00:00:00 2001 From: Devil Date: Sat, 1 Aug 2026 08:43:11 -0400 Subject: [PATCH 1/3] fix: propagate stopAtTop and repCountTiming across exercise transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three WorkoutParameters.copy() transition sites in autoplay omitted stopAtTop and repCountTiming, causing per-exercise 'end at the top' and rep count timing settings to leak from one exercise to all subsequent exercises in a routine. Added explicit propagation from the next RoutineExercise at: - DefaultWorkoutSessionManager.proceedFromSummary (autoplay OFF path) - ActiveSessionEngine.startRestTimer (rest timer → next set) - ActiveSessionEngine.startNextSetOrExercise (autoplay ON path) Fixes #689 --- .../presentation/manager/ActiveSessionEngine.kt | 4 ++++ .../presentation/manager/DefaultWorkoutSessionManager.kt | 2 ++ 2 files changed, 6 insertions(+) diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/ActiveSessionEngine.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/ActiveSessionEngine.kt index da94c4146..1f62e68f8 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/ActiveSessionEngine.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/ActiveSessionEngine.kt @@ -4444,6 +4444,8 @@ class ActiveSessionEngine( isAMRAP = nextIsAMRAP, stallDetectionEnabled = exerciseForNextSet.stallDetectionEnabled, warmupReps = if (nextExerciseIsBodyweight) 0 else Constants.DEFAULT_WARMUP_REPS, + stopAtTop = exerciseForNextSet.stopAtTop, + repCountTiming = exerciseForNextSet.repCountTiming, ) Logger.d { "startRestTimer: Issue #203 - Updated params for next set: ${exerciseForNextSet.exercise.name}, setIdx=$nextSetIdx, isAMRAP=$nextIsAMRAP, nextSetReps=$nextSetReps" } } @@ -4850,6 +4852,8 @@ class ActiveSessionEngine( isAMRAP = nextIsAMRAP, stallDetectionEnabled = nextExercise.stallDetectionEnabled, warmupReps = if (nextIsBodyweight) 0 else Constants.DEFAULT_WARMUP_REPS, + stopAtTop = nextExercise.stopAtTop, + repCountTiming = nextExercise.repCountTiming, ) Logger.d { "startNextSetOrExercise: Issue #203 - progressionKg=${nextExercise.progressionKg}kg for " + diff --git a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/DefaultWorkoutSessionManager.kt b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/DefaultWorkoutSessionManager.kt index 3ac1acb72..e63fc8461 100644 --- a/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/DefaultWorkoutSessionManager.kt +++ b/shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/DefaultWorkoutSessionManager.kt @@ -937,6 +937,8 @@ class DefaultWorkoutSessionManager( selectedExerciseId = nextExercise.exercise.id, isAMRAP = nextIsAMRAP, stallDetectionEnabled = nextExercise.stallDetectionEnabled, + stopAtTop = nextExercise.stopAtTop, + repCountTiming = nextExercise.repCountTiming, ) Logger.d { "proceedFromSummary: Issue #203 - Updated params for next set: ${nextExercise.exercise.name}, setIdx=$nextSetIdx, isAMRAP=$nextIsAMRAP" From 7b2d2860edb442b7183cee149863d8037e7620fa Mon Sep 17 00:00:00 2001 From: Devil Date: Sat, 1 Aug 2026 08:53:52 -0400 Subject: [PATCH 2/3] test: add regression test for per-exercise stopAtTop/repCountTiming propagation Issue #689: Adds a focused regression test that verifies proceedFromSummary() propagates stopAtTop and repCountTiming from the next RoutineExercise when transitioning between exercises (autoplay OFF path). Without the fix from commit 8dc3fcf, the first exercise's values would leak to all subsequent exercises. Addresses Copilot review feedback on PR #690. --- .../manager/DWSMRoutineFlowTest.kt | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt index a403e4a72..aa380fe08 100644 --- a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt +++ b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt @@ -3,6 +3,7 @@ package com.devil.phoenixproject.presentation.manager import com.devil.phoenixproject.domain.model.AppliedRoutineModifier import com.devil.phoenixproject.domain.model.PRType import com.devil.phoenixproject.domain.model.PersonalRecord +import com.devil.phoenixproject.domain.model.RepCountTiming import com.devil.phoenixproject.domain.model.ProgramMode import com.devil.phoenixproject.domain.model.Routine import com.devil.phoenixproject.domain.model.RoutineExercise @@ -856,6 +857,87 @@ class DWSMRoutineFlowTest { harness.cleanup() } + /** + * Issue #689: Regression test for per-exercise stopAtTop/repCountTiming propagation. + * + * Before the fix, WorkoutParameters.copy() at the proceedFromSummary transition + * omitted stopAtTop and repCountTiming, so the first exercise's values leaked + * to all subsequent exercises during autoplay-off routines. + */ + @Test + fun proceedFromSummary_propagatesPerExerciseStopAtTopAndRepCountTiming() = runTest { + val harness = DWSMTestHarness(this) + val exercise0 = TestFixtures.allExercises[0] + val exercise1 = TestFixtures.allExercises[1] + val routine = Routine( + id = "test-689-stopAtTop-repCountTiming", + name = "Issue689 Regression", + exercises = listOf( + RoutineExercise( + id = "re-0-stopAtTop-false", + exercise = exercise0, + orderIndex = 0, + setReps = listOf(10, 10, 10), + weightPerCableKg = 25f, + stopAtTop = false, + repCountTiming = RepCountTiming.BOTTOM, + ), + RoutineExercise( + id = "re-1-stopAtTop-true", + exercise = exercise1, + orderIndex = 1, + setReps = listOf(12, 12, 12), + weightPerCableKg = 15f, + stopAtTop = true, + repCountTiming = RepCountTiming.TOP, + ), + ), + ) + routine.exercises.forEach { harness.fakeExerciseRepo.addExercise(it.exercise) } + advanceUntilIdle() + + harness.dwsm.loadRoutine(routine) + advanceUntilIdle() + + // Verify initial state: exercise 0's values seeded + assertEquals(false, harness.dwsm.coordinator.workoutParameters.value.stopAtTop) + assertEquals(RepCountTiming.BOTTOM, harness.dwsm.coordinator.workoutParameters.value.repCountTiming) + + // Simulate finishing the last set of exercise 0 (autoplay OFF path) + harness.dwsm.coordinator._currentExerciseIndex.value = 0 + harness.dwsm.coordinator._currentSetIndex.value = 2 // last of 3 sets + harness.dwsm.coordinator._workoutState.value = WorkoutState.SetSummary( + metrics = emptyList(), + peakLoadKgPerCable = 20f, + avgLoadKgPerCable = 18f, + repCount = 10, + workingReps = 10, + warmupReps = 0, + ) + + harness.dwsm.proceedFromSummary() + advanceUntilIdle() + + // Assert: exercise 1's stopAtTop and repCountTiming propagated + val params = harness.dwsm.coordinator.workoutParameters.value + assertEquals( + true, + params.stopAtTop, + "stopAtTop must propagate from exercise 1 after proceedFromSummary (Issue #689 regression)", + ) + assertEquals( + RepCountTiming.TOP, + params.repCountTiming, + "repCountTiming must propagate from exercise 1 after proceedFromSummary (Issue #689 regression)", + ) + assertEquals( + exercise1.id, + params.selectedExerciseId, + "Selected exercise should advance to exercise 1", + ) + harness.cleanup() + } + @Test fun goToPreviousExercise_navigatesBackward() = runTest { val harness = DWSMTestHarness(this) From 614b18ed0a101db7255cfcd30f6647ed3bcead25 Mon Sep 17 00:00:00 2001 From: Devil Date: Sat, 1 Aug 2026 09:29:15 -0400 Subject: [PATCH 3/3] test: cover autoplay per-exercise timing transition --- .../manager/DWSMRoutineFlowTest.kt | 64 +++++++++---------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt index aa380fe08..e3b1060cc 100644 --- a/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt +++ b/shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMRoutineFlowTest.kt @@ -858,38 +858,37 @@ class DWSMRoutineFlowTest { } /** - * Issue #689: Regression test for per-exercise stopAtTop/repCountTiming propagation. + * Issue #689: Regression test for the autoplay exercise-boundary transition. * - * Before the fix, WorkoutParameters.copy() at the proceedFromSummary transition - * omitted stopAtTop and repCountTiming, so the first exercise's values leaked - * to all subsequent exercises during autoplay-off routines. + * ActiveSessionEngine.startNextSetOrExercise must replace the preceding exercise's + * stopAtTop and repCountTiming values with the next RoutineExercise's values. */ @Test - fun proceedFromSummary_propagatesPerExerciseStopAtTopAndRepCountTiming() = runTest { + fun startNextSet_propagatesPerExerciseStopAtTopAndRepCountTiming() = runTest { val harness = DWSMTestHarness(this) val exercise0 = TestFixtures.allExercises[0] val exercise1 = TestFixtures.allExercises[1] val routine = Routine( - id = "test-689-stopAtTop-repCountTiming", - name = "Issue689 Regression", + id = "test-689-autoplay-stopAtTop-repCountTiming", + name = "Issue689 Autoplay Regression", exercises = listOf( RoutineExercise( - id = "re-0-stopAtTop-false", + id = "re-0-stopAtTop-true", exercise = exercise0, orderIndex = 0, - setReps = listOf(10, 10, 10), + setReps = listOf(10), weightPerCableKg = 25f, - stopAtTop = false, - repCountTiming = RepCountTiming.BOTTOM, + stopAtTop = true, + repCountTiming = RepCountTiming.TOP, ), RoutineExercise( - id = "re-1-stopAtTop-true", + id = "re-1-stopAtTop-false", exercise = exercise1, orderIndex = 1, - setReps = listOf(12, 12, 12), + setReps = listOf(12), weightPerCableKg = 15f, - stopAtTop = true, - repCountTiming = RepCountTiming.TOP, + stopAtTop = false, + repCountTiming = RepCountTiming.BOTTOM, ), ), ) @@ -899,36 +898,35 @@ class DWSMRoutineFlowTest { harness.dwsm.loadRoutine(routine) advanceUntilIdle() - // Verify initial state: exercise 0's values seeded - assertEquals(false, harness.dwsm.coordinator.workoutParameters.value.stopAtTop) - assertEquals(RepCountTiming.BOTTOM, harness.dwsm.coordinator.workoutParameters.value.repCountTiming) + // Exercise 0 has the values that the report says leaked into later exercises. + assertEquals(true, harness.dwsm.coordinator.workoutParameters.value.stopAtTop) + assertEquals(RepCountTiming.TOP, harness.dwsm.coordinator.workoutParameters.value.repCountTiming) - // Simulate finishing the last set of exercise 0 (autoplay OFF path) + // Simulate a completed rest countdown advancing from exercise 0 to exercise 1. + harness.setActiveSummaryCountdownSeconds(0) harness.dwsm.coordinator._currentExerciseIndex.value = 0 - harness.dwsm.coordinator._currentSetIndex.value = 2 // last of 3 sets - harness.dwsm.coordinator._workoutState.value = WorkoutState.SetSummary( - metrics = emptyList(), - peakLoadKgPerCable = 20f, - avgLoadKgPerCable = 18f, - repCount = 10, - workingReps = 10, - warmupReps = 0, + harness.dwsm.coordinator._currentSetIndex.value = 0 + harness.dwsm.coordinator._workoutState.value = WorkoutState.Resting( + restSecondsRemaining = 0, + nextExerciseName = exercise1.displayName, + isLastExercise = false, + currentSet = 1, + totalSets = 1, ) - harness.dwsm.proceedFromSummary() + harness.dwsm.startNextSet() advanceUntilIdle() - // Assert: exercise 1's stopAtTop and repCountTiming propagated val params = harness.dwsm.coordinator.workoutParameters.value assertEquals( - true, + false, params.stopAtTop, - "stopAtTop must propagate from exercise 1 after proceedFromSummary (Issue #689 regression)", + "stopAtTop must not leak from exercise 0 to exercise 1 (Issue #689 regression)", ) assertEquals( - RepCountTiming.TOP, + RepCountTiming.BOTTOM, params.repCountTiming, - "repCountTiming must propagate from exercise 1 after proceedFromSummary (Issue #689 regression)", + "repCountTiming must not leak from exercise 0 to exercise 1 (Issue #689 regression)", ) assertEquals( exercise1.id,