-
-
Notifications
You must be signed in to change notification settings - Fork 26
fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699
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
Changes from all commits
b88dc44
b7fee41
e834407
e3bb615
80b94a9
1fafc9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ internal enum class RepDropReason { | |
| LEASE_NOT_ACTIVE, | ||
| PRE_CUTOVER_TIMESTAMP, | ||
| TARGET_MISMATCH, | ||
| PROGRESS_BEFORE_EVIDENCE, | ||
| TERMINAL_BEFORE_EVIDENCE, | ||
| } | ||
|
|
||
|
|
@@ -56,12 +57,25 @@ internal class RepNotificationFreshnessGate { | |
| val identity = lease.identity() | ||
| if (notification.isLegacyFormat) return evaluateLegacy(identity, notification) | ||
|
|
||
| val targetMatches = notification.repsSetTotal == 0 || | ||
| notification.repsSetTotal == lease.workingRepTarget | ||
| // Issue #698: Just Lift uses unlimited target semantics (0xFF/252), | ||
| // so the device-reported repsSetTotal will never match the finite UI | ||
| // lease target. Accept only the known unlimited representation or zero | ||
| // for Just Lift; reject stale packets from prior finite-target sets, | ||
| // including one whose target happens to equal the UI lease target. | ||
| val targetMatches = if (lease.isJustLift) { | ||
| notification.repsSetTotal == UNLIMITED_REPS_SET_TOTAL || | ||
| notification.repsSetTotal == 0 | ||
|
9thLevelSoftware marked this conversation as resolved.
|
||
| } else { | ||
| notification.repsSetTotal == 0 || | ||
| notification.repsSetTotal == lease.workingRepTarget | ||
| } | ||
| if (!targetMatches) return RepFreshnessDecision.Drop(RepDropReason.TARGET_MISMATCH) | ||
| if (stateFor(lease) is RepFreshnessState.Armed) return RepFreshnessDecision.Process | ||
|
|
||
| val terminal = lease.workingRepTarget > 0 && | ||
| // Issue #698: Just Lift has no finite rep target, so repsSetCount | ||
| // should never be treated as terminal. Exempt from terminal check. | ||
| val terminal = !lease.isJustLift && | ||
| lease.workingRepTarget > 0 && | ||
| notification.repsSetCount >= lease.workingRepTarget | ||
| val allZero = notification.topCounter == 0 && | ||
| notification.completeCounter == 0 && | ||
|
|
@@ -78,6 +92,9 @@ internal class RepNotificationFreshnessGate { | |
| states[identity] = RepFreshnessState.Armed | ||
| return RepFreshnessDecision.BaselineOnly | ||
| } | ||
| if (lease.isJustLift && hasNonTerminalProgress) { | ||
| return RepFreshnessDecision.Drop(RepDropReason.PROGRESS_BEFORE_EVIDENCE) | ||
|
Comment on lines
+95
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a Just Lift set receives no all-zero rep packet and the user pulls continuously above the velocity threshold, Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in commit 80b94a9: a confirmed |
||
| } | ||
| if (hasNonTerminalProgress) { | ||
| states[identity] = RepFreshnessState.Armed | ||
| return RepFreshnessDecision.Process | ||
|
|
@@ -108,4 +125,9 @@ internal class RepNotificationFreshnessGate { | |
| val executionId: Long, | ||
| val sessionId: String, | ||
| ) | ||
|
|
||
| companion object { | ||
| /** repsSetTotal value the device sends for unlimited/Just Lift/AMRAP sets. */ | ||
| const val UNLIMITED_REPS_SET_TOTAL = 252 | ||
| } | ||
| } | ||
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.
🔥 The Roast: This only arms on a new
Grabbedemission, but the normal Just Lift auto-start path can already be inGrabbedbefore the new lease is activated. The grab that starts the auto-start countdown is emitted before activation;startActiveWorkoutPolling()restarts polling withforAutoStart=false, and the polling engine does not reset the detector, so there is no secondGrabbedevent afterrepFreshnessGate.resetFor(). The first unlimited rep packet is then dropped withPROGRESS_BEFORE_EVIDENCE, and the rest of the set stays at zero reps — a very confident way to count absolutely nothing.🩹 The Fix: After activating the Just Lift lease, inspect the current handle state and call
repFreshnessGate.observeMovement(activeLease)when it isGrabbed(or carry the already-confirmed grab evidence across the auto-start boundary). Add a regression test whereGrabbedis set before workout activation and no post-activation state transition occurs.📏 Severity: warning
Reply with
@kilocode-bot fix itto have Kilo Code address this issue.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.
Fixed in commit 1fafc9d: immediately after activating a Just Lift lease, the engine now carries an already-confirmed
Grabbedhandle state into the freshness gate before polling restarts. Added a zero-less unlimited-warmup lifecycle regression where the confirmed grab predates lease activation.