Skip to content

Cursor/critical bug investigation 38cb - #577

Merged
9thLevelSoftware merged 3 commits into
mainfrom
cursor/critical-bug-investigation-38cb
Jun 17, 2026
Merged

Cursor/critical bug investigation 38cb#577
9thLevelSoftware merged 3 commits into
mainfrom
cursor/critical-bug-investigation-38cb

Conversation

@9thLevelSoftware

Copy link
Copy Markdown
Owner

No description provided.

phoenix-bot and others added 3 commits June 17, 2026 16:26
… (Issue #572)

When a routine has two adjacent RoutineExercise entries that share the same
physical exercise but have different programMode (e.g. 'Sumo Belt Squat 2x8
OldSchool' followed by 'Sumo Belt Squat 1x8 TUT'), the routine-advance
logic was unconditionally sending a fresh 0x04 BLE CONFIG frame for the
mode change. The Vitruvian firmware de-energises the cable on a mode
change, which the user reported as 'the set deloads weight but the screen
still shows the set weight'.

This change introduces a 'same-exercise continuation' branch in
ActiveSessionEngine.startNextSetOrExercise:
  * RoutineFlowManager.isSameExercise() matches adjacent entries by name,
    plus id when both ids are non-null (id check skipped when either side
    is null for legacy / unlinked exercise data).
  * When getNextStep advances to the next entry and isSameExercise() is
    true, the consumer:
      - keeps the on-screen programMode label aligned with the new entry
        while carrying forward the current programMode to the firmware
        (so the cable is NOT de-energised mid-movement);
      - applies the per-set weight from the new entry's
        setWeightsPerCableKg[nextSetIdx];
      - skips the fresh startWorkout() (no fresh 0x04 CONFIG frame) and
        goes to SetReady so the user explicitly starts the TUT finisher,
        at which point the fresh CONFIG is sent at the right time;
      - does NOT re-seed rack defaults, does NOT reset the rep counter,
        does NOT re-initialise the warm-up phase (it is still the same
        physical exercise).
  * Genuinely different adjacent exercises, the superset branch, and
    same-entry set advance (preserves manual rest-screen weight/rep
    edits) are all unchanged.

Acceptance criteria covered by new regression tests in
DWSMRoutineFlowTest (section G. Same-exercise continuation):
  1. (0, 0) -> (0, 1) stays inside Lunge 2x8, not jumping to Lunge TUT.
  2. Full 'Legs' sequence: (0,1) -> (1,0) -> (2,0) -> (2,1) -> (3,0)
     -> (4,0) -> null.
  3. No same-exercise adjacency (existing fixture) is unchanged.
  4. Different ids with same name do NOT merge.
  5. Null ids (legacy / unlinked data) are still detected as same
     exercise.
  6. Superset branch interleaving is unchanged for same-exercise entries
     inside a superset.
  7. isSameExercise is false for genuinely different adjacent exercises.

No schema change. No BLE packet format change. No firmware change. No
superset-branch change. No enterSetReady change. No new UI de-duplication
at routine edit time.

Fixes #572
…creen stall

Issue #572 same-exercise continuation called enterSetReady while workoutState
remained Resting after autoplay rest completion. ActiveWorkoutScreen only
navigates to SetReady when workoutState is Idle, so users were stranded on the
rest UI. Tapping Skip Rest re-invoked startNextSetOrExercise and skipped the
deferred TUT finisher set entirely.

Set workoutState to Idle before enterSetReady and add a regression test.

Co-authored-by: Devil <9thLevelSoftware@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 17, 2026 22:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@9thLevelSoftware
9thLevelSoftware merged commit 2200bc3 into main Jun 17, 2026
13 checks passed
@9thLevelSoftware
9thLevelSoftware deleted the cursor/critical-bug-investigation-38cb branch June 17, 2026 22:38

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses navigation and double-advance issues during same-exercise continuations by ensuring the workout state transitions to Idle and restricting continuations to adjacent linear exercises. Comprehensive unit tests have been added to verify these fixes. The reviewer suggested a code improvement in ActiveSessionEngine.kt to simplify the boolean logic, leveraging Kotlin's smart-casting and avoiding an intermediate variable.

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.

Comment on lines +4417 to 4424
val isAdjacentLinearExercise =
nextExIdx == coordinator._currentExerciseIndex.value + 1 &&
currentExercise?.supersetId == null &&
nextExercise.supersetId == null
val isSameExerciseContinuation = isChangingExercise &&
currentExercise != null &&
isAdjacentLinearExercise &&
flowDelegate?.isSameExercise(currentExercise, nextExercise) == true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

We can simplify this logic by combining isAdjacentLinearExercise directly into isSameExerciseContinuation. This avoids creating an unnecessary intermediate variable, leverages Kotlin's smart-casting for currentExercise (so we don't need the safe-call ?. operator on supersetId), and utilizes short-circuit evaluation to prevent redundant checks when currentExercise is null.

Suggested change
val isAdjacentLinearExercise =
nextExIdx == coordinator._currentExerciseIndex.value + 1 &&
currentExercise?.supersetId == null &&
nextExercise.supersetId == null
val isSameExerciseContinuation = isChangingExercise &&
currentExercise != null &&
isAdjacentLinearExercise &&
flowDelegate?.isSameExercise(currentExercise, nextExercise) == true
val isSameExerciseContinuation = isChangingExercise &&
currentExercise != null &&
nextExIdx == coordinator._currentExerciseIndex.value + 1 &&
currentExercise.supersetId == null &&
nextExercise.supersetId == null &&
flowDelegate?.isSameExercise(currentExercise, nextExercise) == true

@kilo-code-bot

kilo-code-bot Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

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.

This PR fixes Issue #572 with surgical precision — adding adjacency checks to prevent same-exercise continuation from incorrectly triggering when (a) an intervening exercise was skipped, or (b) superset member transitions occur. The logic change in ActiveSessionEngine.kt is well-reasoned, and the three new tests in DWSMRoutineFlowTest.kt comprehensively cover the edge cases. The workoutState.Idle fix is the kind of detail that only surfaces after someone gets bitten in production.

The only change I could even snark about is the new Superset import in the test file, but it's legitimately needed for the new superset test. Zero complaints.

📊 Overall: Like finding a unicorn in production — meticulously crafted, well-tested, and ready to ship.

Files Reviewed (2 files)

Reviewed by laguna-m.1-20260312:free · 1,650,036 tokens

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants