Skip to content

fix: exempt AMRAP leases from rep freshness gate (#700) #701

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.

Sign up for GitHub

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
9thLevelSoftware merged 3 commits into from
Aug 16, 2026
+109 −2
Merged

fix: exempt AMRAP leases from rep freshness gate (#700) #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b88dc44
fix: exempt Just Lift leases from target mismatch and terminal checks…
Aug 16, 2026
f288cef
fix: exempt AMRAP leases from rep freshness gate (Issue #700)
Aug 16, 2026
4989b06
fix: restrict AMRAP target exemption to unlimited target (target=0)
Aug 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions ...Main/kotlin/com/devil/phoenixproject/presentation/manager/RepNotificationFreshnessGate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,22 @@ internal class RepNotificationFreshnessGate {
val identity = lease.identity()
if (notification.isLegacyFormat) return evaluateLegacy(identity, notification)

val targetMatches = notification.repsSetTotal == 0 ||
// Issue #698/#700: Just Lift and AMRAP with target=0 use unlimited
// target semantics (0xFF/252), so the device-reported repsSetTotal
// will never match the finite UI lease target. Exempt both from
// target equality check. AMRAP with a finite target (>0) must still
// match — only unlimited AMRAP gets the exemption.
val targetMatches = lease.isJustLift ||
(lease.isAmrap && lease.workingRepTarget == 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

val terminal = lease.workingRepTarget > 0 &&
// Issue #698/#700: Just Lift and AMRAP have no finite rep target,
// so repsSetCount should never be treated as terminal.
val terminal = !(lease.isJustLift || lease.isAmrap) &&
lease.workingRepTarget > 0 &&
notification.repsSetCount >= lease.workingRepTarget
val allZero = notification.topCounter == 0 &&
notification.completeCounter == 0 &&
Expand Down
97 changes: 97 additions & 0 deletions .../kotlin/com/devil/phoenixproject/presentation/manager/RepNotificationFreshnessGateTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,103 @@ class RepNotificationFreshnessGateTest {
)
}

// --- Issue #698: Just Lift target mismatch exemption ---

@Test
fun `just lift lease accepts repsSetTotal 252 despite finite UI target`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 10, cutover = 1_000L).copy(isJustLift = true)

// First packet establishes baseline and arms
assertEquals(RepFreshnessDecision.BaselineOnly, gate.evaluate(lease, modernPacket(timestamp = 1_001L)))
assertEquals(RepFreshnessState.Armed, gate.stateFor(lease))

// repsSetTotal=252 (unlimited) should NOT be dropped as TARGET_MISMATCH
assertEquals(
RepFreshnessDecision.Process,
gate.evaluate(lease, modernPacket(repsSetCount = 1, repsSetTotal = 252, timestamp = 1_002L)),
)
}

@Test
fun `just lift lease does not treat repsSetCount as terminal`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 3, cutover = 1_000L).copy(isJustLift = true)

// repsSetCount=3 >= workingRepTarget=3 would be terminal for finite,
// but Just Lift should process it normally after baseline
assertEquals(RepFreshnessDecision.BaselineOnly, gate.evaluate(lease, modernPacket(timestamp = 1_001L)))
assertEquals(
RepFreshnessDecision.Process,
gate.evaluate(lease, modernPacket(repsSetCount = 3, repsSetTotal = 252, timestamp = 1_002L)),
)
}

@Test
fun `finite lease still rejects mismatched repsSetTotal after fix`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 3, cutover = 1_000L) // isJustLift = false

assertEquals(
RepFreshnessDecision.Drop(RepDropReason.TARGET_MISMATCH),
gate.evaluate(lease, modernPacket(repsSetCount = 1, repsSetTotal = 252, timestamp = 1_001L)),
)
}

// --- Issue #700: AMRAP target mismatch exemption (mirrors #698 Just Lift tests) ---

@Test
fun `amrap lease accepts repsSetTotal 252 despite finite UI target`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 0, cutover = 1_000L).copy(isAmrap = true)

// First packet establishes baseline and arms
assertEquals(RepFreshnessDecision.BaselineOnly, gate.evaluate(lease, modernPacket(timestamp = 1_001L)))
assertEquals(RepFreshnessState.Armed, gate.stateFor(lease))

// repsSetTotal=252 (unlimited) should NOT be dropped as TARGET_MISMATCH for AMRAP
assertEquals(
RepFreshnessDecision.Process,
gate.evaluate(lease, modernPacket(repsSetCount = 1, repsSetTotal = 252, timestamp = 1_002L)),
)
}

@Test
fun `amrap lease does not treat repsSetCount as terminal`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 0, cutover = 1_000L).copy(isAmrap = true)

// AMRAP should never treat repsSetCount as terminal, same as Just Lift
assertEquals(RepFreshnessDecision.BaselineOnly, gate.evaluate(lease, modernPacket(timestamp = 1_001L)))
assertEquals(
RepFreshnessDecision.Process,
gate.evaluate(lease, modernPacket(repsSetCount = 5, repsSetTotal = 252, timestamp = 1_002L)),
)
}

@Test
fun `finite amrap lease still rejects mismatched repsSetTotal`() {
val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 3, cutover = 1_000L).copy(isAmrap = true)

// Even with isAmrap=true, a mismatched finite repsSetTotal should be rejected
assertEquals(
RepFreshnessDecision.Drop(RepDropReason.TARGET_MISMATCH),

@kilo-code-bot kilo-code-bot Bot Aug 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 The Roast: This assertion is going to fail in CI with the confidence of someone who shipped without running tests. You wrote copy(isAmrap = true) on line 234, then expect the gate to drop for TARGET_MISMATCH. But line 62 of the gate short-circuits target matching whenever isAmrap is true — targetMatches is unconditionally true, so we skip the drop and fall through. Then terminal is forced false by the AMRAP exemption (line 70), allZero is false (repsSetCount=1), hasNonTerminalProgress is true, and the gate returns Process. The "even with isAmrap=true, mismatched finite repsSetTotal should be rejected" comment is right about the intent and wrong about the implementation — AMRAP gets a blanket exemption, no finite-target validation when isAmrap=true. The PR description bragged "Local Java runtime unavailable; tests will be validated by CI" — yep, CI is going to validate straight into a red ❌.

🩹 The Fix: Mirror the JustLift version at line 190 — drop copy(isAmrap = true) from line 234 so it actually exercises the non-exempt path. Or delete this test outright since finite lease still rejects mismatched repsSetTotal after fix already covers the non-exempt case.

📏 Severity: critical

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

gate.evaluate(lease, modernPacket(repsSetCount = 1, repsSetTotal = 4, timestamp = 1_001L)),
)
}

@Test
fun `pre-cutover amrap packet is still rejected`() {

@kilo-code-bot kilo-code-bot Bot Aug 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 The Roast: This test exists in a quantum superposition — it both adds coverage and adds nothing. The pre-cutover drop at lines 51–54 of the gate runs before any isAmrap is consulted, so this assertion holds for any lease regardless of isAmrap. The JustLift mirror set has no equivalent for exactly this reason. You wrote it anyway. Like backup paperwork for a backup.

🩹 The Fix: Delete this test. Pre-cutover behavior is already covered by pre-cutover packet drop prevents stale notification processing at line 11 and is independent of isAmrap.

📏 Severity: suggestion

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

val gate = RepNotificationFreshnessGate()
val lease = activeLease(target = 0, cutover = 1_000L).copy(isAmrap = true)

assertEquals(
RepFreshnessDecision.Drop(RepDropReason.PRE_CUTOVER_TIMESTAMP),
gate.evaluate(lease, modernPacket(repsSetCount = 1, repsSetTotal = 252, timestamp = 999L)),
)
}

private fun activeLease(target: Int, cutover: Long) = ExecutionLease(
executionId = 1L,
sessionId = "session-a",
Expand Down
Loading