fix: stall-detection overhaul — Echo deload no longer ends sets, racked pauses safe, VBT baseline leak closed (e2e audit + fixes)#654
Conversation
Follow-up to #652 / PR #653 and the 2026-07-14 Echo session report (sets force-ended at 6 and ~3 reps on Harder/Hardest with VBT auto-end disabled). Nine findings, ranked. Root cause of the report: DELOAD_OCCURRED is a routine firmware event in Echo mode (Echo levels are defined by deload windows) but is treated as cable release and arms the 5s auto-stop stall timer (F1). Compounding: VBT velocity-loss baseline contaminated by firmware warm-up reps (F2), Phase 35C warm-up-set early return skips biomech/VBT resets (F3), global stall-detection toggle does not govern routine sets (F6). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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 FindingsTraced the full F1→F8 chain end-to-end against the diff:
No correctness or safety findings. Ponytail ReviewStandard-library/native:
Shrink:
Test:
Ponytail: Lean already. Ship. Ponytail net: 0 lines. Suggested Minimal PatchNo patch needed. Final Merge GuidanceCan 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 Files Reviewed (12 files)
🤖 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 FindingsNo correctness or safety findings. Ponytail ReviewThe audit is a new standalone document under Ponytail: Lean already. Ship. Ponytail net: 0 lines. Suggested Minimal PatchNo patch needed. Final Merge GuidanceCan merge as-is. The follow-up fixes (F1/F3/F2/F6 at minimum) belong in their own PRs against the cited 📊 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)
Reviewed by minimax-m3 · Input: 48.1K · Output: 9.7K · Cached: 572.8K |
- F1: ignore DELOAD_OCCURRED in Echo mode — Echo levels are defined by the firmware's deload window, so the event fires routinely mid-set and must not arm the 5s auto-stop stall timer (root cause of sets force-ending at 6/3 reps on Harder/Hardest). - #652/#653 absorbed: a completed working rep resets the shared stall timer in handleRepNotification (supersedes PR #653). - F3: startWorkout() resets 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 into the working set. - F4: velocity-armed stall countdowns now require handles in use (maxPosition > STALL_MIN_POSITION) to arm and to continue, re-checked per sample — a racked pause between reps no longer ends a standard set. New stallArmedByDeload flag exempts deload-armed countdowns (genuine cable release retracts to ~0mm and must still auto-stop); a deload upgrades an existing velocity-armed countdown. - F7: deferAutoStopDeadlineMs moved to WorkoutCoordinator (@volatile); WorkoutCoordinator.resetAutoStopState() is the single reset for all auto-stop/stall/defer fields; ASE and RFM delegate to it. - F8: releasing the handles during the verbal-cue defer window clears the deadline so auto-stop resumes promptly instead of waiting out 30s. Regression tests: Echo deload ignored / Echo velocity stall kept, Issue 652 rep-cancels-stall, F3 set-start reset, 4x F4 arm-source scenarios, F8 clear-on-release, coordinator reset unit test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq
The global UserPreferences.stallDetectionEnabled was consumed by no workout path and had no wired UI toggle — every workout honors the per-exercise RoutineExercise.stallDetectionEnabled or the Just Lift per-session toggle. Its only effect was incorrectly gating the 'Auto-End on Velocity Loss' row in Settings, implying a global control that did not exist. Removed: the preference field, persistence key/load/setter (PreferencesManager), SettingsManager/MainViewModel passthroughs, the SettingsTab parameter and gating (auto-end row is now always enabled), and the fake's override. JustLiftDefaults.stallDetectionEnabled and the per-exercise flag are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq
F2 and F9 withdrawn (totalReps excludes warm-up reps; stall fields already @volatile), F5 resolved via #653 absorb + F4 continuation check, and per-finding resolution status for the fixes on this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq
9thLevelSoftware
changed the title
…icked identifiers The iOS Test Target Compile CI job failed on WorkoutCoordinatorAutoStopResetTest with 'Name contains illegal characters: ","'. JVM targets accept commas in backticked test names; Kotlin/Native does not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ee4mJkzUkJTbwEUvMCSSaq
2 participants
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.mdFixes
DELOAD_OCCURREDfires 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.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.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 bypassedhandleSetCompletion's reset block, leaking the warm-up set's velocity baseline (at ~50% weight) into the working set.stallArmedByDeloadflag 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).stallDetectionEnabledpreference — 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.deferAutoStopDeadlineMsmoved toWorkoutCoordinator(@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).Audit corrections (addendum §6)
getRepCount().totalRepsexcludes 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.@Volatile.Testing
WorkoutCoordinatorAutoStopResetTest.:shared:testAndroidHostTest: 2451 tests, 0 failures, 0 errors, 0 skipped.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