AMRAP ROM reps not registering in JustLift mode

[9thLevelSoftware]

Bug Report: AMRAP ROM Reps Not Registering

Reporter: shredness_10860 (Discord)
Type: Bug
Repo: Mobile (project-phoenix-mp)
Device: Pixel 9 XL Pro
OS: Android, v17
App Version: 0.9.6

Summary

Range of Motion (ROM) reps do not register during AMRAP (As Many Reps As Possible) sets in JustLift mode. There is no audible cue and no rep count increment on screen. However, set load does kick in at the "fourth" rep. Even at full load, no reps are counted, the user remains stuck on the ROM screen, and the failure does not trigger end of set.

Steps to Reproduce

1. Open JustLift (which is AMRAP by default)
2. Set any weight
3. Perform ROM reps

Result: ROM reps never complete but weight will kick in. Set never fails because no reps are counted.

Expected Behavior

• ROM reps should register with an audible cue and screen uptick
• Reps should be counted and displayed during AMRAP sets
• Set should eventually fail/complete when the user cannot perform more reps

Actual Behavior

• No audible cue on ROM rep completion
• No rep count increment on screen during AMRAP sets
• Set load kicks in at "fourth" rep but no reps are counted
• Stuck on ROM screen — no way to progress or end the set
• Failure does not trigger end of set

Comparison (Working Behavior)

When using any exercise with explicit rep counts (non-AMRAP), the routine behaves as expected: ROM reps register, subsequent reps are counted, and the set completes normally.

Impact

This bug makes JustLift/AMRAP mode completely non-functional for tracking workout progress. Users cannot track their rep performance in this mode.

Attachments

No attachments recorded.

---

Original Discord Report

<@1502654170031849644> ROM error when set to AMRAP

Device: Pixel 9 XL Pro
OS: Android, v17
App: Version 0.9.6

range of motion reps not registering in app for AMRAP sets. no audible cue, no uptick on screen. however, set load will kick in at "fourth" rep. still no reps are counted at all even at full load, still stuck on ROM screen, failure does not trigger end of set.

steps to reproduce: Open JustLift (which is AMRAP by default), set any weight. ROM reps never complete but weight will kick in, set never fails because no reps are counted.

Compare to: open any exercise, set reps, execute. routine behaves as expected. ROM reps register, subsequent reps counted.

[9thLevelSoftware]

Additional info from Discord thread:

correction: ANY exercise with AMRAP set is affected. justlift was given as an example

— added by ticket bot

[9thLevelSoftware]

RCA: AMRAP / JustLift ROM rep notifications dropped by freshness gate

Recreation status

• Reproduction artifact: phoenix-bug-recreations/700/bug-recreation.md (full evidence matrix at phoenix-bug-recreations/700/evidence/)
• Recreation outcome: REPRODUCED at code level (deterministic). Live code at ade389f (main).
• Reporter clarification: the reporter's Discord reply (msg 1538512997041840140) widened the bug from "JustLift" to ANY AMRAP set. This is captured in the GitHub issue body and the recreation evidence (amrap-routine-trace.md).

Root cause (one-line)

RepNotificationFreshnessGate.evaluate() deterministically drops every rep notification whose repsSetTotal != lease.workingRepTarget, but for both JustLift and routine AMRAP sets the Vitruvian machine sends back repsSetTotal = 252 (UNLIMITED_REPS_SET_TOTAL, 0xFF on the wire) — which never equals the lease's finite UI target (10 for JustLift, 0 for routine AMRAP). The bug is one || short of recognizing the AMRAP / JustLift exemption.

Root cause in detail

The gate lives in shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/RepNotificationFreshnessGate.kt:

// evaluate(), lines 59-61 (current, pre-RCA main)
val targetMatches = notification.repsSetTotal == 0 ||
    notification.repsSetTotal == lease.workingRepTarget
if (!targetMatches) return RepFreshnessDecision.Drop(RepDropReason.TARGET_MISMATCH)

The Armed short-circuit at line 62 sits after the target check, so once the gate returns Drop(TARGET_MISMATCH) the rep is gone before RepCounterFromMachine.process() can run. The cascade is:

1. acceptRepNotification() receives machine packet (repsSetTotal = 252).
2. repFreshnessGate.evaluate() returns Drop(TARGET_MISMATCH).
3. handleRepNotification() is never called → repCounter.process() never runs.
4. warmupReps stays at 0 → warmup phase never completes → UI stays on the ROM screen.
5. coordinator._repCount never advances → no audible cue (audio is keyed off rep events).
6. Auto-stop depends on rep count progressing past the warmup gate (isWarmupGateOpenForAutoStop()), so the set never ends.

Both reported and widened paths hit the same line:

Lease mode	lease.isJustLift	lease.isAmrap	lease.workingRepTarget	notification.repsSetTotal	Gate outcome
JustLift (default)	true	true	10	252	Drop(TARGET_MISMATCH)
AMRAP exercise in routine	false	true	0	252	Drop(TARGET_MISMATCH)
Finite rep exercise	false	false	10	10	Process (passes)

The BlePacketFactory already encodes 0xFF (252) on the wire for both isJustLift || isAMRAP (BlePacketFactory.kt:369), so the on-the-wire protocol is consistent — the freshness gate is the only place that doesn't recognize the sentinel.

Why this is the binding RCA, not a hypothesis

• The recreation agent verified REPRO-01..REPRO-07 at code level (lease target, machine repsSetTotal, gate decision, warmup, working rep, PR fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699 coverage, and routine AMRAP).
• ExecutionLease already exposes isJustLift: Boolean and isAmrap: Boolean (WorkoutExecutionGuard.kt:59-70) — the missing check is purely in the gate logic.
• BlePacketFactory already confirms the on-wire target for AMRAP is 0xFF (252) — the gate's lease.workingRepTarget mismatch is a UI-side artifact, not a protocol mismatch.
• A precedent fix is already open and reviewed for JustLift only: PR fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699 (fix/issue-698-just-lift-rep-freshness-gate, HEAD b7fee41). It adds an isJustLift exemption but misses isAmrap, which is why issue AMRAP ROM reps not registering in JustLift mode #700 still reproduces after fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699 lands.

Fix direction (binding for the downstream fixer)

Extend PR #699's JustLift exemption to also cover lease.isAmrap. Treat the two flags as the same condition for the gate decision; the on-wire protocol already does this.

Exact code change

File: shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/manager/RepNotificationFreshnessGate.kt

Replace the targetMatches block (lines 59-61) and the terminal block (lines 64-65) with:

// Issue #698 + #700: Just Lift and AMRAP use unlimited target semantics
// (0xFF/252 on the wire), so the device-reported repsSetTotal will never
// match the finite UI lease target. Accept only the known unlimited
// representation or zero for these modes; reject stale packets from prior
// finite-target sets.
val isUnlimitedMode = lease.isJustLift || lease.isAmrap
val targetMatches = (isUnlimitedMode &&
        (notification.repsSetTotal == UNLIMITED_REPS_SET_TOTAL ||
            notification.repsSetTotal == 0)) ||
    notification.repsSetTotal == 0 ||
    notification.repsSetTotal == lease.workingRepTarget
if (!targetMatches) return RepFreshnessDecision.Drop(RepDropReason.TARGET_MISMATCH)
if (stateFor(lease) is RepFreshnessState.Armed) return RepFreshnessDecision.Process

// AMRAP / Just Lift have no finite rep target, so repsSetCount must
// never be treated as terminal for them.
val terminal = !isUnlimitedMode &&
    lease.workingRepTarget > 0 &&
    notification.repsSetCount >= lease.workingRepTarget

(UNLIMITED_REPS_SET_TOTAL = 252 is added by PR #699 in the same companion-object block; reuse it.)

The fix is intentionally a single predicate (isUnlimitedMode) applied to both checks so JustLift and routine AMRAP get the same semantics — they're the same condition on the wire.

Test pattern (use PR #699's tests as the template)

Extend shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/manager/RepNotificationFreshnessGateTest.kt by mirroring PR #699's four @Test cases for isAmrap = true:

1. routine AMRAP lease accepts repsSetTotal 252 despite zero UI target (lease workingRepTarget = 0, isAmrap = true, isJustLift = false; baseline → Process for repsSetTotal = 252).
2. routine AMRAP lease does not treat repsSetCount as terminal (lease workingRepTarget = 0, isAmrap = true; baseline → Process for repsSetCount = 5).
3. routine AMRAP lease rejects stale finite repsSetTotal from prior execution (baseline → Drop(TARGET_MISMATCH) for repsSetTotal = 15).
4. finite AMRAP-flagged lease still rejects mismatched repsSetTotal (lease isAmrap = false, repsSetTotal = 252 → Drop(TARGET_MISMATCH)).

Keep PR #699's existing JustLift tests unchanged.

Acceptance criteria

A correct fix MUST:

• Accept post-cutover modern packets with repsSetTotal = 252 for both isJustLift = true and isAmrap = true leases.
• NOT treat repsSetCount as terminal for either isJustLift = true or isAmrap = true leases.
• Still reject nonzero mismatched repsSetTotal for finite-target leases (isJustLift = false, isAmrap = false).
• Leave pre-cutover, invalidated, and non-current execution packets rejected.
• Extend PR fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699's existing test suite (do not duplicate or rename its JustLift tests); add the four AMRAP test cases listed above.
• Pass ./gradlew :shared:testAndroidHostTest --tests "*RepNotificationFreshnessGateTest*" --console=plain and ./gradlew :shared:testAndroidHostTest --tests "*DWSMWorkoutLifecycleTest*" --console=plain.

A correct fix MUST NOT:

• Change BlePacketFactory, the BLE command framing, or JustLiftScreen parameters.
• Widen the exemption to any other mode (e.g. isBodyweight, isTimedCable). The isTimedCable exemption already exists at line 86 of the test file but is gated on repsSetTotal == 0, not on any lease flag — leave it alone.
• Introduce a new RepDropReason; reuse TARGET_MISMATCH and TERMINAL_BEFORE_EVIDENCE.
• Touch WorkoutExecutionGuard or ExecutionSeed; the flags already exist.

Regression test target

Primary: RepNotificationFreshnessGateTest (the four new AMRAP cases above, plus PR #699's JustLift suite).

Secondary (regression sweep): DWSMWorkoutLifecycleTest, WorkoutExecutionGuardTest, Issue687StaleWorkSuppressionTest, WorkoutExitPersistenceTest, WorkoutMachineTeardownTest, RepCounterFromMachineTest. None of these should require changes; they should all still pass.

Non-goals

• Changing how workingRepTarget is computed for AMRAP sets (the 0 for routine AMRAP and 10 for JustLift are intentional UI defaults; do not "fix" them).
• Removing the freshness gate or weakening its other guards (pre-cutover, legacy, terminal-before-evidence).
• Speculative AMRAP-style exemptions for flags other than isJustLift / isAmrap.
• Addressing the secondary HandleState.Grabbed → observeMovement race that PR fix: exempt Just Lift from rep freshness gate target mismatch (#698) #699's earlier revisions attempted (it was dropped from the PR head). That is a separate concern; file a follow-up issue if it remains a problem during the merge-gate audit.

Implementation handoff

The downstream bugfix agent should check out PR #699's branch (fix/issue-698-just-lift-rep-freshness-gate), rebase onto current main, apply the patch above, update the tests, and either reopen PR #699 or open a new PR (e.g. fix/issue-700-amrap-rep-freshness-gate) that explicitly extends the exemption. The PR body must include Fixes #700.

— RCA posted by the Project Phoenix GPT-5.6 Terra xhigh RCA lead. Coordinates: rca_owner=gpt-5.6-terra-xhigh, rca_model=gpt-5.6-terra, rca_provider=openai-codex, fix_driver=gpt56_terra_rca.

[9thLevelSoftware]

Architecture Review: APPROVED

Reviewer: phoenixarchitect (arch_review stage)
Issue: #700 — AMRAP ROM reps not registering
RCA reference: comment

RCA Verification

Root cause confirmed at source (ade389f, main): RepNotificationFreshnessGate.evaluate() lines 59-61 deterministically drops every rep notification where repsSetTotal != lease.workingRepTarget. For AMRAP/JustLift sets the Vitruvian machine sends repsSetTotal=252 (0xFF on the wire), which never equals the finite UI target (10 for JustLift, 0 for routine AMRAP). The Drop(TARGET_MISMATCH) fires before RepCounterFromMachine.process() runs, cascading into frozen warmup, no rep count, no audio cue, and no auto-stop — exactly matching the report.

PR #699 (open, fix/issue-698-just-lift-rep-freshness-gate) already exempts lease.isJustLift at both the target check and terminal check. Routine AMRAP sets hit the same gate with the same 0xFF packet but are NOT exempted — this is the gap.

Architectural Assessment

Breaking changes / migration: None. The fix extends an existing boolean predicate from isJustLift to isJustLift || isAmrap. No schema, no API, no persistence, no Supabase/edge function involvement.

Pattern alignment: Strong. The isJustLift || isAMRAP pattern is already the established convention across the codebase:

• BlePacketFactory.kt:369 — wire encoding: if (isJustLift || isAMRAP) 0xFF
• RepCounterFromMachine.kt:364,594 — terminal rep check: !isJustLift && !isAMRAP
• ActiveSessionEngine.kt:477,496,1201,1219,1243,1264 — auto-stop/final-rep: isJustLift || isAMRAP
• WorkoutCommandValidator.kt:51,81 — validation: !isJustLift && !isAMRAP
• WorkoutHud.kt:617,666 — UI display: !isJustLift && !isAMRAP

The freshness gate is the only site that uses isJustLift alone for unlimited-target semantics. Extending it to isJustLift || isAmrap makes it consistent with all 15+ existing call sites.

Data model: ExecutionLease (WorkoutExecutionGuard.kt:59-70) already has isAmrap: Boolean populated from seedParams.isAMRAP at lease creation (ActiveSessionEngine.kt:2874). No new fields needed.

Cross-repo risk: None. This is mobile-only. Portal/sync/Supabase are not affected — AMRAP is a session-execution concept, not a persistence schema concern.

Scope: Minimal and focused. One predicate change in one file (RepNotificationFreshnessGate.kt), plus four mirror test cases in RepNotificationFreshnessGateTest.kt covering: isAmrap=true with target=0 + repsSetTotal=252 passes; terminal exemption; stale finite rejection; finite AMRAP mismatch still rejected. This does not risk broadening into unrelated refactors.

Compatibility: Safe for both PR #699 (issue #698) and this issue. The fix should be applied on top of PR #699's branch or merged into it, since both address the same gate with the same pattern. No conflict with finite-rep or timed-cable paths — those continue to use the else branch.

Verdict

APPROVED. The fix direction is technically sound, architecturally consistent with existing conventions, minimal in scope, and carries no cross-system risk. Proceed to implementation.