-
-
Notifications
You must be signed in to change notification settings - Fork 26
fix: defer AMRAP/stall auto-stop after verbal VBT cue when autoEndOnVelocityLoss=false (#649) #650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
72b8952
fix: defer AMRAP/stall auto-stop after verbal VBT cue when autoEndOnV…
1e4c0c3
fix(#649): @Volatile for cross-thread defer + add deadline window
d59b44e
fix(#649): single-source deadline + resetAutoStopState clears defer
34897f5
fix(#649): kill derived flag, single-source deadline
0d0f4ad
fix(#649): narrow arm race window — drop stale-callback arms after se…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
172 changes: 172 additions & 0 deletions
172
...lin/com/devil/phoenixproject/presentation/manager/VerbalEncouragementAutoStopDeferTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,172 @@ | ||
| package com.devil.phoenixproject.presentation.manager | ||
|
|
||
| import kotlin.test.Test | ||
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertFalse | ||
| import kotlin.test.assertTrue | ||
|
|
||
| /** | ||
| * Issue #649: while a verbal VBT cue is playing and the user has VBT auto-end OFF, | ||
| * neither the AMRAP position-based nor the velocity-stall auto-stop may end the set. | ||
| * | ||
| * Mirrors the velocity-threshold state machine from VbtAutoEndTest / VerbalEncouragementGateTest. | ||
| * Keeps the logic testable without spinning up the 17+ dependency ActiveSessionEngine. | ||
| * | ||
| * Single source of truth is a @Volatile Long deadline — 0L means no defer. The | ||
| * tracker mirrors that pattern: an `isDeferActive(nowMs)` predicate derived from | ||
| * the deadline only, with the flag computed lazily inside the predicate. | ||
| */ | ||
| private const val VERBAL_ENCOURAGEMENT_DEFER_WINDOW_MS_TRACKER = 30_000L | ||
|
|
||
| private class VerbalEncouragementAutoStopDeferTracker( | ||
| private val autoEndOnVelocityLoss: Boolean, | ||
| ) { | ||
| private var deferDeadlineMs: Long = 0L | ||
|
|
||
| fun onRepCompleted(shouldStopSet: Boolean, nowMs: Long) { | ||
| if (shouldStopSet) { | ||
| if (deferDeadlineMs == 0L && !autoEndOnVelocityLoss) { | ||
| // Verbal cue fires once per set when VBT auto-end is OFF. | ||
| deferDeadlineMs = nowMs + VERBAL_ENCOURAGEMENT_DEFER_WINDOW_MS_TRACKER | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun onCompletedWorkingRep() { | ||
| // A completed working rep proves the user is back in motion; let normal | ||
| // AMRAP / stall auto-stop resume by zeroing the deadline. | ||
| deferDeadlineMs = 0L | ||
| } | ||
|
|
||
| /** Returns true if the defer is still active at `nowMs`. */ | ||
| fun isDeferActive(nowMs: Long): Boolean { | ||
| val deadline = deferDeadlineMs | ||
| if (deadline == 0L) return false | ||
| if (nowMs >= deadline) { | ||
| deferDeadlineMs = 0L | ||
| return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| /** Mirrors resetAutoStopState(): every new-set reset clears the defer. */ | ||
| fun onResetAutoStopState() { | ||
| deferDeadlineMs = 0L | ||
| } | ||
|
|
||
| /** Test seam: read the published deadline (mirrors the @Volatile field). */ | ||
| fun deferDeadlineSnapshot(): Long = deferDeadlineMs | ||
|
|
||
| /** Test seam: true when no defer is armed (one-shot is fired unless VBT auto-end). */ | ||
| fun armEligible(): Boolean = deferDeadlineMs == 0L && !autoEndOnVelocityLoss | ||
| } | ||
|
|
||
| class VerbalEncouragementAutoStopDeferTest { | ||
|
|
||
| @Test | ||
| fun `autoEnd off - first velocity-threshold rep arms the defer with a future deadline`() { | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 0L) | ||
|
|
||
| assertEquals(30_000L, tracker.deferDeadlineSnapshot()) | ||
| assertTrue(tracker.isDeferActive(nowMs = 0L)) | ||
| assertTrue(tracker.isDeferActive(nowMs = 29_999L)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `autoEnd on - first velocity-threshold rep does NOT arm the defer`() { | ||
| // With VBT auto-end ON, the existing two-consecutive-rep behavior decides set end; | ||
| // the defer guard would only suppress that and must NOT be armed. | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = true) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 0L) | ||
|
|
||
| assertEquals(0L, tracker.deferDeadlineSnapshot()) | ||
| assertFalse(tracker.isDeferActive(nowMs = 0L)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `next completed working rep clears the defer regardless of deadline remaining`() { | ||
| // Verbal cue armed at t=5s, user completes a working rep at t=10s. | ||
| // The defer must clear immediately, even though the 30s window hasn't expired, | ||
| // so AMRAP / stall auto-stop can fire normally on subsequent metrics. | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 5_000L) | ||
| assertTrue(tracker.isDeferActive(nowMs = 10_000L)) | ||
|
|
||
| tracker.onCompletedWorkingRep() | ||
|
|
||
| assertEquals(0L, tracker.deferDeadlineSnapshot()) | ||
| assertFalse( | ||
| tracker.isDeferActive(nowMs = 10_000L), | ||
| "After a completed working rep the defer must clear so AMRAP/stall can fire again", | ||
| ) | ||
| assertFalse( | ||
| tracker.isDeferActive(nowMs = 29_999L), | ||
| "Re-arm must require another velocity-threshold-triggered cue, not automatic re-derivation", | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun `autoEnd off - non-threshold reps do not arm the defer`() { | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = false, nowMs = 0L) | ||
| tracker.onRepCompleted(shouldStopSet = false, nowMs = 0L) | ||
|
|
||
| assertEquals(0L, tracker.deferDeadlineSnapshot()) | ||
| assertFalse(tracker.isDeferActive(nowMs = 0L)) | ||
| assertTrue(tracker.armEligible(), "Cue hasn't fired yet, so arm remains eligible") | ||
| } | ||
|
|
||
| @Test | ||
| fun `defer stays active within the cue window`() { | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 10_000L) | ||
|
|
||
| assertTrue(tracker.isDeferActive(nowMs = 20_000L)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `defer expires at the cue window deadline`() { | ||
| // User rackets handles mid-set after the cue and never completes a working rep. | ||
| // Once the deadline passes, the defer must release so AMRAP/stall auto-stop can fire. | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 0L) | ||
|
|
||
| assertTrue(tracker.isDeferActive(nowMs = 29_999L), "Just before deadline still defers") | ||
| assertFalse( | ||
| tracker.isDeferActive(nowMs = 30_001L), | ||
| "Past the deadline the defer must clear so auto-stop can fire normally", | ||
| ) | ||
| assertEquals(0L, tracker.deferDeadlineSnapshot(), "Expiry must zero the deadline") | ||
| } | ||
|
|
||
| @Test | ||
| fun `resetAutoStopState clears the defer for the next set`() { | ||
| // Verbal cue armed the defer mid-set. Before the 30s deadline the user | ||
| // skips / restarts. The next active set must not inherit the defer — | ||
| // otherwise checkAutoStop() would suppress AMRAP/stall auto-stop there too. | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 10_000L) | ||
| assertTrue(tracker.isDeferActive(nowMs = 20_000L)) | ||
|
|
||
| tracker.onResetAutoStopState() | ||
|
|
||
| assertEquals(0L, tracker.deferDeadlineSnapshot()) | ||
| assertTrue(tracker.armEligible(), "Next set must allow a fresh cue to arm again") | ||
| assertFalse( | ||
| tracker.isDeferActive(nowMs = 20_000L), | ||
| "After resetAutoStopState() the new set must allow normal auto-stop behavior", | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun `arm publishes future deadline that a metrics-thread reader sees immediately`() { | ||
| // Single-source-of-truth guarantee: the deadline field is the gate. | ||
| // Even a reader that observes the field the instant the cue fires sees | ||
| // a future deadline, never the stale 0L value. | ||
| val tracker = VerbalEncouragementAutoStopDeferTracker(autoEndOnVelocityLoss = false) | ||
| tracker.onRepCompleted(shouldStopSet = true, nowMs = 5_000L) | ||
| assertEquals(35_000L, tracker.deferDeadlineSnapshot()) | ||
| assertTrue(tracker.isDeferActive(nowMs = 5_000L)) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When biomechanics work from a completed set runs late, this assignment can publish a fresh 30s defer after
startWorkout()has already reset state for the next set.processBiomechanicsForRep()launches on the shared scope andcheckVelocityThreshold()does not verify that the analyzed rep still belongs to the current set, so in a quick set transition/autostart path an old verbal-cue callback can make the new active set'scheckAutoStop()suppress AMRAP/stall auto-stop until the timeout or another rep clears it. Capture and validate a set/session generation before arming the deadline; a simple Active-state check is not enough once the next set has started.Useful? React with 👍 / 👎.