fix(#656): Set Summary (unlimited) + Next Set now advances per tap, not silently skipping to End Routine

[9thLevelSoftware]

Fixes #656

Bug

With Set Summary configured to "unlimited" (autoplay OFF) and per-set rest times that differ, tapping "Next Set" on the Set Summary screen did not advance to the next set. Each tap silently advanced the set index; after N taps (N = total sets), the Summary page changed to "End Routine" and exited the exercise, skipping every middle set's SetReady transition.

Root cause

DefaultWorkoutSessionManager.proceedFromSummary() autoplay OFF branch (line 903-) advanced _currentSetIndex / _currentExerciseIndex and called enterSetReady() (which sets routineFlowState = SetReady) but never transitioned workoutState out of SetSummary. The companion routine-complete branch (line 892) and the autoplay ON branch (line 977 + rest timer) both transition workoutState; the autoplay OFF branch was the only one missing the transition.

ActiveWorkoutScreen.kt (lines 295-306) gates navigation to the SetReady screen on !isWorkoutActive, and isWorkoutActive includes WorkoutState.SetSummary. With workoutState stuck in SetSummary, the gate never fires and the screen stays mounted on SetSummary while the index advances underneath. After N taps, getNextStep() returns null and the routine-complete branch fires, displaying "End Routine".

Fix

Add coordinator._workoutState.value = WorkoutState.Idle immediately before enterSetReady(nextExIdx, nextSetIdx) in the autoplay OFF branch. This mirrors the routine-complete branch pattern (line 892) and makes isWorkoutActive = false so the nav gate unmounts SetSummary and routes to SetReady.

One-line state-machine fix. No UI changes, no autoplay ON behavior changes.

Tests

• New Issue656ReproTest in shared/src/commonTest (AndroidHostTest):
  1. proceedFromSummary_unlimitedSummaryDoesNotNavigateOutOfSetSummary — asserts that after the first "Next Set" tap, routineFlowState is SetReady AND workoutState has left SetSummary (the bug fingerprint).
  2. proceedFromSummary_unlimitedSecondTapAdvancesFurtherAndEventuallyCompletes — asserts the last-set tap drives routineFlowState.Complete (End Routine), matching the reporter's symptom.
• All existing presentation.manager.* AndroidHostTests still pass (180+ tests, 0 failures).

Acceptance criteria (from RCA)

1. Unlimited Set Summary + Next Set advances to SetReady per tap. ✓
2. All sets are traversed. ✓
3. Routine ends cleanly on the last set. ✓
4. Autoplay ON unaffected. ✓ (unchanged code path)

[gemini-code-assist]

Code Review

This pull request addresses Issue #656 by resetting the workout state to Idle when transitioning to the SetReady screen, which ensures the UI correctly unmounts the set summary. It also adds a reproduction test suite (Issue656ReproTest.kt) to verify this behavior. The review feedback suggests reversing the order of state updates in DefaultWorkoutSessionManager.kt to prevent potential UI race conditions, and replacing reflection-based class name assertions in the tests with type-safe is checks to avoid issues with Kotlin Multiplatform and obfuscation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[kilo-code-bot]

Code Review Roast 🔥

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything, and the diff turns out to be a one-line state-machine fix that mirrors a pattern already living ten lines above it. Whoever traced this bug to the missing workoutState transition instead of throwing a LaunchedEffect band-aid on the UI side has my grudging respect.

📊 Overall: Like a 3-bullet PR description that actually matches the diff. Rare breed.

Correctness / Safety Findings

No correctness or safety findings. The fix mirrors line 892 verbatim (routine-complete branch) and is consistent with the autoplay ON branch (line 989). Both regression tests assert behavior, not implementation details.

Ponytail Review

shared/src/commonTest/kotlin/com/devil/phoenixproject/Issue656ReproTest.kt:L51-76: test-shrink: the Exercise/Routine/RoutineExercise block is duplicated verbatim across both tests (~22 lines). Extract a private fun issue656Routine(): Routine helper or use WorkoutStateFixtures.createTestRoutine(...) plus per-set overrides.

shared/src/commonTest/kotlin/com/devil/phoenixproject/Issue656ReproTest.kt:L112-172: test-shrink (optional): proceedFromSummary_unlimitedSecondTapAdvancesFurtherAndEventuallyCompletes only asserts routineFlowState == Complete, which the pre-fix routineComplete branch already produced. It documents the user-reported symptom but does not prove the fix. Test 1 alone covers the actual change. Safe to delete (~75 lines), safe to keep as documentation.

shared/src/commonTest/kotlin/com/devil/phoenixproject/Issue656ReproTest.kt:1: yagni (optional): the 21-line preamble comment duplicates the PR description. A 2-3 line note is enough.

Ponytail net: -22 lines (dedupe), up to -97 lines if the symptom-replication test goes.

Suggested Minimal Patch

No patch needed for correctness. Optional cleanup: extract the duplicated routine fixture into a private helper.

Final Merge Guidance

Can merge as-is. Ponytail suggestions are optional cleanup and do not block the fix.

Files Reviewed (2 files)

• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/DefaultWorkoutSessionManager.kt - 0 issues
• shared/src/commonTest/kotlin/com/devil/phoenixproject/Issue656ReproTest.kt - 0 blocking issues (optional dedupe)

Fix these issues in Kilo Cloud

---

_{Reviewed by minimax-m3 · Input: 45.2K · Output: 5.5K · Cached: 356.4K}