fix: stall-detection overhaul — Echo deload no longer ends sets, racked pauses safe, VBT baseline leak closed (e2e audit + fixes)

[9thLevelSoftware]

E2e audit of the stall-detection / auto-stop pipeline plus implementation of every actionable finding. Follow-up to #652 / PR #653 and the 2026-07-14 Echo session report (set 1 on Hard completed 10/10; set 2 on Harder force-ended at 6 reps; set 3 on Hardest ended after ~3 reps — with Auto-End on Velocity Loss disabled).

Audit report (with a Corrections & Resolutions addendum): docs/audits/2026-07-14-stall-detection-e2e-audit.md

Fixes

• F1 (root cause): Echo levels are defined by the firmware's deload window (HARD = 1.0s @ 50 mm/s, HARDER = 1.25s @ 40 mm/s — per the Issue Echo Mode stuck on Warm Up 1/3 screen, weight pinned at 8.8 lb (Android 16, OnePlus 15, v0.9.2) #553 comment), so DELOAD_OCCURRED fires routinely mid-set as the athlete fatigues. The deload collector now ignores it entirely in Echo mode; Echo sets keep velocity-stall and release/position protection.
• fix(#652): reset shared stall timer on completed working rep #653 absorbed: a completed working rep resets the shared stall timer in handleRepNotification (Issue Echo VBT state accumulates across progressive sets and stops later sets unexpectedly (related to #649) #652). PR fix(#652): reset shared stall timer on completed working rep #653 is superseded by this PR and can be closed.
• F3: startWorkout() now resets the biomechanics engine, VBT one-shot flags, and rep boundary timestamps at every set start — the Phase 35C variable warm-up fast path bypassed handleSetCompletion's reset block, leaking the warm-up set's velocity baseline (at ~50% weight) into the working set.
• F4: velocity-armed stall countdowns now require handles in use (>10 mm) to arm and to continue (re-checked per sample) — a ≥5s racked pause between reps no longer force-ends a standard set. A new stallArmedByDeload flag keeps genuine cable-release auto-stop working (retracting cables must not cancel a deload-armed countdown; a deload also upgrades an existing velocity-armed one).
• F6: removed the dead global stallDetectionEnabled preference — it was consumed by no workout path and had no wired UI toggle; its only effect was incorrectly gating the "Auto-End on Velocity Loss" settings row (now always enabled). Per-exercise and Just Lift toggles are unchanged and remain the only controls.
• F7: deferAutoStopDeadlineMs moved to WorkoutCoordinator (@volatile); WorkoutCoordinator.resetAutoStopState() is the single reset for all auto-stop/stall/defer state (ASE and RoutineFlowManager both delegate — the RFM copy previously couldn't clear the defer deadline).
• F8: releasing the handles during the 30s verbal-cue defer window clears it immediately, so a genuine set-end auto-stops promptly.

Audit corrections (addendum §6)

• F2 withdrawn: getRepCount().totalReps excludes warm-up reps, so there is no intra-set warm-up contamination of the VBT baseline — the only real vector was the F3 cross-set leak.
• F9 withdrawn: the stall/auto-stop fields were already @Volatile.
• F5 resolved via the fix(#652): reset shared stall timer on completed working rep #653 absorb + F4 per-sample position re-check (no dedicated change).

Testing

• 9 new regression tests (Echo deload ignored / Echo velocity-stall kept, Issue Echo VBT state accumulates across progressive sets and stops later sets unexpectedly (related to #649) #652 rep-cancels-stall, F3 set-start reset, 4× F4 arm-source scenarios, F8 clear-on-release) + WorkoutCoordinatorAutoStopResetTest.
• Full :shared:testAndroidHostTest: 2451 tests, 0 failures, 0 errors, 0 skipped.
• All pre-existing deload/stall gating tests (Issue [Bug]: auto-stop no longer working #256 pending-rep pair, deload expiry, Issue [Bug]: What did I actually lift? #267) stay green.

What to expect on hardware

Echo sets at Harder/Hardest no longer end ~5s after a firmware deload; racking mid-set >5s no longer ends a standard set; genuinely releasing the cables still auto-stops in ~5s; AMRAP/Just Lift racked-handle behavior (2.5s position countdown) is unchanged; after a VBT verbal cue, releasing the handles ends the set promptly instead of waiting out the 30s window.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq

[gemini-code-assist]

Code Review

This pull request adds a comprehensive end-to-end audit document (docs/audits/2026-07-14-stall-detection-e2e-audit.md) investigating issues with the stall detection and auto-stop pipeline. The audit outlines the signal flow, ranks nine key findings (including critical firmware deload issues in Echo mode and velocity-loss baseline contamination), reconstructs the reported incident, and provides a prioritized list of recommended fixes. There are no review comments, so I have no feedback to provide.

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

The audit didn't just finger-point and walk away — it actually shipped every actionable finding (F1, F3, F4, F6, F7, F8), absorbed #653, and withdrew F2/F9 with receipts. In a codebase where "I'll add a TODO and explain in a doc" is the default response to a systemic bug, this is the version-control equivalent of "show me the receipts."

Correctness / Safety Findings

Traced the full F1→F8 chain end-to-end against the diff:

• F1 (Echo deload ignored): clean early-return gated by params.isEchoMode. Velocity-stall path is untouched → dedicated test confirms Echo velocity-stall still arms. ✓
• F3 (set-start reset): reset block moved into startWorkout() covering both Phase 35C branches. Comment explicitly notes idempotence with the existing handleSetCompletion() reset. ✓
• F4 (racked-handle cancel + deload upgrade): the || hasMeaningfulRange latch is gone, the per-sample re-check is gated by !stallArmedByDeload, and the upgrade branch preserves the existing stallStartTime (doesn't restart the timer). ✓
• F6 (dead preference removal): clean sweep across interface, impl, ViewModel, NavGraph, SettingsTab param, and FakePreferencesManager. No orphaned references. ✓
• F7 (single reset): deferAutoStopDeadlineMs and stallArmedByDeload now live on the coordinator with @Volatile; both ASE and RFM delegate. ✓
• F8 (release clears defer): correctly placed in the handle-state collector before the Just-Lift auto-stop branch, so transient blips can't skip the 2.5–5s countdown. ✓
• Issue Echo VBT state accumulates across progressive sets and stops later sets unexpectedly (related to #649) #652 absorb: resetStallTimer() on completed working rep — the absorbed fix is a 1-liner with a targeted regression test. ✓

No correctness or safety findings.

Ponytail Review

Standard-library/native:

• All resets are pull-delegates to coordinator.resetAutoStopState() — no duplicated hand-copied field lists. The RFM hand-copy (Audit F7) was the actual bug; the fix is the Ponytail.

Shrink:

• handleSetCompletion's redundant biomechanicsEngine.reset() block can now be collapsed with the new startWorkout() block (the comment notes they're idempotent). Could be one block in one method. Optional cleanup, not required — leaving both is the more conservative call when timing of "summary displayed" matters.

Test:

• The new tests are tight: each one asserts exactly the bug class it was written for, mostly just null/notNull/WorkoutState.Active. No 60-line mock-worship tests.

Ponytail: Lean already. Ship.

Ponytail net: 0 lines.

Suggested Minimal Patch

No patch needed.

Final Merge Guidance

Can merge as-is. The audit addendum correctly identifies which F2/F9 claims didn't survive verification against the actual code — those withdrawals save the PR from looking like cargo-cult self-justification.

📊 Overall: Like a git blame that actually apologizes — eight fixes shipped, two findings honestly withdrawn with line-level evidence, and a test suite grew by 7+ targeted regressions. The author read their own audit.

Files Reviewed (12 files)

• docs/audits/2026-07-14-stall-detection-e2e-audit.md
• shared/src/commonMain/kotlin/com/devil/phoenixproject/data/preferences/PreferencesManager.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/domain/model/UserPreferences.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/ActiveSessionEngine.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/RoutineFlowManager.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/SettingsManager.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/WorkoutCoordinator.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/navigation/NavGraph.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/SettingsTab.kt
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/viewmodel/MainViewModel.kt
• shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/DWSMWorkoutLifecycleTest.kt
• shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/WorkoutCoordinatorAutoStopResetTest.kt
• shared/src/commonTest/kotlin/com/devil/phoenixproject/testutil/FakePreferencesManager.kt

---

🤖 Generated with Claude Code

Previous Review Summary (commit 2670fba)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 2670fba)

Verdict: No Issues Found | Recommendation: Merge

Oh look, a docs-only PR that knows it's docs-only. In a codebase where "I'll just slip in a refactor while I'm here" is the natural state of being, this author actually wrote 134 lines of markdown and zero lines of production code. Restraint so disciplined it almost looks suspicious.

Correctness / Safety Findings

No correctness or safety findings.

Ponytail Review

The audit is a new standalone document under docs/audits/ with nine ranked findings, a scope diagram, an incident reconstruction table, an explicit assessment of #653, and a recommended fix order. Every section pulls weight: scope defines the signal flow and the four end-set paths; the ranking table is the actual deliverable; §4 directly answers "why isn't #653 enough?"; §5 sequences the follow-ups. Nothing speculative, no future-proofing, no boilerplate preamble. The inline () line-range citations (ActiveSessionEngine.kt:400-437, etc.) are doing real work — they're the cross-references a follow-up PR will need to land fixes against. Trimming this would just make the postmortem less useful.

Ponytail: Lean already. Ship.

Ponytail net: 0 lines.

Suggested Minimal Patch

No patch needed.

Final Merge Guidance

Can merge as-is. The follow-up fixes (F1/F3/F2/F6 at minimum) belong in their own PRs against the cited main locations, not retro-edited into this audit.

📊 Overall: Like a postmortem written by the engineer who actually stayed until the root cause was found, not the one who left at "VBT was on." The headline finding alone — that the user was right, VBT wasn't the killer — earns its 134 lines.

Files Reviewed (1 file)

• docs/audits/2026-07-14-stall-detection-e2e-audit.md

---

_{Reviewed by minimax-m3 · Input: 48.1K · Output: 9.7K · Cached: 572.8K}