Fix UI-boundary defects; run tests on the Android variant too

Two defects found reviewing the UI boundary before Compose work: 1. HumanAgent.awaiting was a plain mutable property written by the game coroutine and read by the UI — a data race, and invisible to Compose. It also exposed DecisionContext, which holds a live Seat whose fields mutate as the hand proceeds, so even a safe read could observe torn state. Replaced with an immutable DecisionOffer published through a StateFlow. A test mutates the live seat after publication and asserts the offer does not change. 2. STREET_COMPLETE was emitted after dealing the new street but before the round state was reset, so a flop snapshot carried pre-flop currentBet and committedThisRound — the UI would have painted last street's chips in front of every player alongside the new board. The reset is now prepareRound(), called before publishing. Verified: with the ordering reverted the new test fails with currentBet 10 on the flop. Also: - HumanAgent.cancel() is now covered directly; the previous test only cancelled the coroutine running act(). cancel() and submit() both report whether anything was actually pending. - Android host tests enabled via withHostTestBuilder, so the shared suite runs against the Android variant instead of the AAR merely compiling. No librsvg needed for card assets: sips rasterizes the SVGs directly at exact 2:3 dimensions, court cards and patterned backs included. Tests: 45 -> 48, now green on both jvmTest and testAndroidHostTest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-25 15:33:17 -04:00
parent 7f82251d86
commit eefcd5966c
5 changed files with 173 additions and 19 deletions
@@ -4,7 +4,7 @@ plugins {
 }

 kotlin {
-    // JVM target drives tests and the headless tuning simulator.
+    // JVM target drives the headless tuning simulator.
    jvm()

    // The modern KMP Android integration. `androidTarget()` is the older path.
@@ -12,6 +12,12 @@ kotlin {
        namespace = "com.jsjdesigns.poker.engine"
        compileSdk = 37
        minSdk = 26
+
+        // Run the shared tests against the Android variant too, not just compile
+        // it — otherwise the AAR is only ever proven to build.
+        withHostTestBuilder {}.configure {
+            isIncludeAndroidResources = true
+        }
    }

    sourceSets {
@@ -0,0 +1,42 @@
+package com.jsjdesigns.poker.game
+
+/**
+ * An immutable snapshot of the decision a human player is being asked to make.
+ *
+ * [DecisionContext] cannot cross the UI boundary: it holds a live [Seat] whose
+ * fields the game coroutine mutates as the hand proceeds, so anything the UI read
+ * from it would be a data race and would not recompose. This copies out only what
+ * is needed to render an action bar.
+ */
+data class DecisionOffer(
+    val handNumber: Int,
+    val street: Street,
+    val seat: Int,
+    val hole: List<Int>,
+    val board: List<Int>,
+    val pot: Int,
+    val toCall: Int,
+    val minRaiseTo: Int,
+    val maxRaiseTo: Int,
+    val stack: Int,
+    val canCheck: Boolean,
+    val canRaise: Boolean,
+) {
+    /** Convenience for a call-or-check button label. */
+    val callAmount: Int get() = toCall
+}
+
+fun DecisionContext.toOffer(): DecisionOffer = DecisionOffer(
+    handNumber = handNumber,
+    street = street,
+    seat = seat.index,
+    hole = hole.toList(),
+    board = board.toList(),
+    pot = pot,
+    toCall = toCall,
+    minRaiseTo = minRaiseTo,
+    maxRaiseTo = maxRaiseTo,
+    stack = stack,
+    canCheck = canCheck,
+    canRaise = canRaise,
+)
@@ -1,42 +1,53 @@
 package com.jsjdesigns.poker.game

 import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asStateFlow
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock

 /**
 * A [PlayerAgent] driven by the UI rather than by code.
 *
- * When it is this player's turn, [act] publishes the decision on offer and then
+ * When it is this player's turn, [act] publishes an immutable [DecisionOffer] and
 * suspends until [submit] delivers a choice. Cancelling the coroutine running the
- * hand — a screen closing, a game being abandoned — completes the pending wait
+ * hand — a screen closing, a game abandoned — completes the pending wait
 * exceptionally rather than leaving it parked forever.
+ *
+ * [offer] is a `StateFlow` rather than a plain property because it is written by
+ * the game coroutine and read by the UI thread: a bare `var` would be both a data
+ * race and invisible to Compose.
 */
 class HumanAgent : PlayerAgent {

    private val lock = Mutex()
    private var pending: CompletableDeferred<Action>? = null

-    /** The decision currently awaiting input, or null when it is not our turn. */
-    var awaiting: DecisionContext? = null
-        private set
+    private val _offer = MutableStateFlow<DecisionOffer?>(null)
+
+    /** The decision awaiting input, or null when it is not this player's turn. */
+    val offer: StateFlow<DecisionOffer?> = _offer.asStateFlow()
+
+    val isAwaitingInput: Boolean get() = _offer.value != null

    override suspend fun act(ctx: DecisionContext): Action {
        val deferred = CompletableDeferred<Action>()
        lock.withLock {
            check(pending == null) { "already awaiting a decision for this agent" }
            pending = deferred
-            awaiting = ctx
        }
+        // Published after the deferred is installed, so a UI that reacts instantly
+        // to the offer always finds something able to receive its submission.
+        _offer.value = ctx.toOffer()
+
        return try {
            deferred.await()
        } finally {
            // Runs on normal completion AND on cancellation, so the agent is never
            // left believing it is still waiting.
-            lock.withLock {
-                pending = null
-                awaiting = null
-            }
+            _offer.value = null
+            lock.withLock { pending = null }
        }
    }

@@ -49,10 +60,10 @@ class HumanAgent : PlayerAgent {
        deferred.complete(action)
    }

-    /** Abandons the pending decision, releasing [act] with a cancellation. */
-    suspend fun cancel(cause: Throwable = IllegalStateException("hand abandoned")) {
-        lock.withLock { pending?.completeExceptionally(cause) }
-    }
-
-    val isAwaitingInput: Boolean get() = awaiting != null
+    /** Abandons the pending decision, releasing [act] with [cause]. */
+    suspend fun cancel(cause: Throwable = IllegalStateException("hand abandoned")): Boolean =
+        lock.withLock {
+            val deferred = pending ?: return@withLock false
+            deferred.completeExceptionally(cause)
+        }
 }
@@ -207,6 +207,7 @@ class Table(
        postBlinds()
        dealHoleCards()
        currentStreet = Street.PREFLOP
+        prepareRound(Street.PREFLOP)
        emit(TableSnapshot.Phase.DEALT)

        var street = Street.PREFLOP
@@ -239,6 +240,9 @@ class Table(
                Street.RIVER -> { finished = true; Street.RIVER }
            }
            currentStreet = street
+            // Clear the previous street's betting BEFORE publishing, so the new
+            // board never arrives alongside last street's chips.
+            prepareRound(street)
            emit(TableSnapshot.Phase.STREET_COMPLETE)
        }

@@ -346,7 +350,15 @@ class Table(
    private fun mayRaise(seat: Seat): Boolean =
        seat.lastActedAtBet < 0 || (currentBet - seat.lastActedAtBet) >= minRaiseSize

-    private suspend fun runBettingRound(street: Street, firstSeat: Int) {
+    /**
+     * Clears per-round betting state for [street].
+     *
+     * Called *before* the new-street snapshot is published, not lazily at the top
+     * of the betting round: otherwise a flop snapshot still carries pre-flop
+     * `currentBet` and `committedThisRound`, and the UI paints stale chips in
+     * front of every player.
+     */
+    private fun prepareRound(street: Street) {
        for (s in seats) {
            s.committedThisRound = 0
            s.hasActed = false
@@ -366,7 +378,10 @@ class Table(
            currentBet = 0
        }
        minRaiseSize = bigBlind
+    }

+    /** Runs the round. [prepareRound] must already have been called for [street]. */
+    private suspend fun runBettingRound(street: Street, firstSeat: Int) {
        if (seats.count { it.canAct } == 0) return

        var i = firstSeat
@@ -141,7 +141,7 @@ class SnapshotAndHumanAgentTest {
        val decision = async { human.act(context(seat)) }

        while (!human.isAwaitingInput) yield()
-        assertNotNull(human.awaiting, "the UI must be able to see what is on offer")
+        assertNotNull(human.offer.value, "the UI must be able to see what is on offer")
        assertTrue(human.submit(Action(ActionType.CALL, 10)))

        assertEquals(ActionType.CALL, decision.await().type)
@@ -197,3 +197,83 @@ class SnapshotAndHumanAgentTest {
        assertEquals(listOf(1), result.winners, "the bot wins once the human folds")
    }
 }
+
+class HumanOfferAndStreetStateTest {
+
+    private fun context(seat: Seat) = DecisionContext(
+        street = Street.PREFLOP,
+        seat = seat,
+        board = IntArray(0),
+        pot = 15,
+        toCall = 10,
+        minRaiseTo = 20,
+        maxRaiseTo = 500,
+        activeOpponents = 1,
+        seatsActingAfter = 0,
+        bigBlind = 10,
+        history = emptyList(),
+        handNumber = 1,
+        bettingReopened = true,
+    )
+
+    /** Codex note: the earlier test cancelled the coroutine, never cancel() itself. */
+    @Test
+    fun `cancel releases the waiting agent and reports whether anything was pending`() = runTest {
+        val human = HumanAgent()
+        val seat = Seat(0, "You", 500, human)
+
+        assertFalse(human.cancel(), "nothing pending yet")
+
+        val decision = async { runCatching { human.act(context(seat)) } }
+        while (!human.isAwaitingInput) yield()
+
+        assertTrue(human.cancel(), "cancel reports that it released a pending wait")
+        assertTrue(decision.await().isFailure, "act must complete exceptionally")
+        assertNull(human.offer.value, "the offer must be cleared")
+        assertFalse(human.isAwaitingInput)
+    }
+
+    @Test
+    fun `the published offer is an immutable copy, not a live seat`() = runTest {
+        val human = HumanAgent()
+        val seat = Seat(0, "You", 500, human)
+        seat.hole = intArrayOf(0, 5)
+
+        val decision = async { human.act(context(seat)) }
+        while (!human.isAwaitingInput) yield()
+        val offer = human.offer.value!!
+
+        // Mutating the live seat the way the game coroutine would must not be
+        // visible through the already-published offer.
+        seat.stack = 1
+        seat.committedThisRound = 999
+        assertEquals(500, offer.stack, "offer must not alias live seat state")
+        assertEquals(listOf(0, 5), offer.hole)
+
+        human.submit(Action(ActionType.FOLD))
+        decision.await()
+    }
+
+    @Test
+    fun `a new street snapshot carries no stale betting from the previous street`() = runTest {
+        val seen = mutableListOf<TableSnapshot>()
+        val seats = listOf(Seat(0, "A", 500, Caller()), Seat(1, "B", 500, Caller()))
+        Table(
+            seats, 5, 10, Random(1),
+            StackedDeck.of(listOf("Ah Ad", "Kh Kd"), "2c 7d 9s Jc 3h"),
+            observer = { seen += it },
+        ).playHand()
+
+        val streetStarts = seen.filter { it.phase == TableSnapshot.Phase.STREET_COMPLETE }
+        assertTrue(streetStarts.isNotEmpty(), "there should be new-street snapshots")
+        for (snap in streetStarts) {
+            assertEquals(0, snap.currentBet, "a fresh street starts with no bet outstanding")
+            assertTrue(
+                snap.seats.all { it.committedThisRound == 0 },
+                "chips from the previous street must be swept into the pot first",
+            )
+        }
+        // The pot must still reflect everything committed so far.
+        assertTrue(streetStarts.all { it.pot > 0 }, "the pot carries forward")
+    }
+}