diff --git a/CLAUDE.md b/CLAUDE.md index baba18b..971d0b9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -72,7 +72,9 @@ export JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" version, setup preference, session/hand counts, baseline agreement, and stable review-category counters. It never stores hole cards, boards, opponents, or raw action histories. Storage mutations are serialised because hand completion - and coach analysis run on different coroutines. + and coach analysis run on different coroutines. History is non-critical: an + unreadable/newer schema or failed commit falls back to in-memory counters and + disables persistence for that process rather than crashing or overwriting data. ## Testing notes @@ -142,7 +144,9 @@ export JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" - Versioned player history survives process restarts in a small aggregate `SharedPreferences` store. The setup screen restores the last player/coach preference and shows cross-session agreement/review trends only after at least - ten coached decisions; smaller samples are labelled as insufficient. + ten coached decisions; smaller samples are labelled as insufficient. Schema + reads go through an explicit migration dispatcher; add the migration branch + before incrementing `PLAYER_HISTORY_SCHEMA_VERSION`. - Gradle emits an `archives` deprecation from the Kotlin Multiplatform plugin's own `jvm()` target registration — upstream in Kotlin 2.2.10, not our build. - Not built yet: LLM persona layer. diff --git a/app/src/main/java/com/jsjdesigns/poker/PlayerHistoryStore.kt b/app/src/main/java/com/jsjdesigns/poker/PlayerHistoryStore.kt index 66aca96..7d587ef 100644 --- a/app/src/main/java/com/jsjdesigns/poker/PlayerHistoryStore.kt +++ b/app/src/main/java/com/jsjdesigns/poker/PlayerHistoryStore.kt @@ -3,6 +3,7 @@ package com.jsjdesigns.poker import android.annotation.SuppressLint import android.content.Context import android.content.SharedPreferences +import android.util.Log import com.jsjdesigns.poker.bot.CoachingReview import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.sync.Mutex @@ -16,20 +17,29 @@ interface PlayerHistoryStore { suspend fun recordCoachReview(review: CoachingReview): PlayerHistory } +internal interface PlayerHistoryPersistence { + fun read(): PlayerHistory + fun write(history: PlayerHistory): Boolean +} + /** - * Small versioned aggregate store. + * Serialised read-modify-write with a deliberately non-fatal failure policy. * - * SharedPreferences is appropriate here because the complete schema is a few - * scalar counters. Mutations are serialised and committed on Dispatchers.IO so a - * hand completion and an asynchronous coach review cannot overwrite each other. + * History is coaching metadata, never game-critical state. If data is corrupt, + * belongs to a newer app schema, or cannot be written, the current process keeps + * a fresh in-memory history and stops touching durable storage. In particular, + * an older sideloaded APK neither crashes nor overwrites a newer schema. */ -class SharedPreferencesPlayerHistoryStore(context: Context) : PlayerHistoryStore { - private val preferences = - context.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE) +internal class ResilientPlayerHistoryStore( + private val persistence: PlayerHistoryPersistence, + private val failureReporter: (message: String, error: RuntimeException?) -> Unit = { _, _ -> }, +) : PlayerHistoryStore { private val mutex = Mutex() + private var cachedHistory: PlayerHistory? = null + private var persistenceAvailable = true override suspend fun load(): PlayerHistory = withContext(Dispatchers.IO) { - mutex.withLock { preferences.readHistory() } + mutex.withLock { currentHistory() } } override suspend fun recordSession( @@ -46,55 +56,128 @@ class SharedPreferencesPlayerHistoryStore(context: Context) : PlayerHistoryStore private suspend fun mutate(block: (PlayerHistory) -> PlayerHistory): PlayerHistory = withContext(Dispatchers.IO) { mutex.withLock { - val updated = block(preferences.readHistory()) - check(preferences.writeHistory(updated)) { "could not persist player history" } + val updated = block(currentHistory()) + cachedHistory = updated + if (persistenceAvailable) { + try { + if (!persistence.write(updated)) { + disablePersistence("Could not commit player history", null) + } + } catch (error: RuntimeException) { + disablePersistence("Could not write player history", error) + } + } updated } } - private companion object { - const val PREFERENCES_NAME = "player_history" - const val KEY_SCHEMA = "schema_version" - const val KEY_PLAYER_NAME = "player_name" - const val KEY_COACH_ENABLED = "coach_enabled" - const val KEY_SESSIONS = "sessions_started" - const val KEY_HANDS = "hands_completed" - const val KEY_DECISIONS = "decisions_reviewed" - const val KEY_ALIGNED = "baseline_aligned" - - fun SharedPreferences.readHistory(): PlayerHistory { - val storedSchema = getInt(KEY_SCHEMA, PLAYER_HISTORY_SCHEMA_VERSION) - require(storedSchema <= PLAYER_HISTORY_SCHEMA_VERSION) { - "player history schema $storedSchema is newer than supported " + - PLAYER_HISTORY_SCHEMA_VERSION + private fun currentHistory(): PlayerHistory { + cachedHistory?.let { return it } + val loaded = if (persistenceAvailable) { + try { + persistence.read() + } catch (error: RuntimeException) { + disablePersistence("Could not read player history", error) + PlayerHistory() } - return PlayerHistory( - playerName = getString(KEY_PLAYER_NAME, "").orEmpty(), - coachEnabled = getBoolean(KEY_COACH_ENABLED, false), - sessionsStarted = getLong(KEY_SESSIONS, 0), - handsCompleted = getLong(KEY_HANDS, 0), - decisionsReviewed = getLong(KEY_DECISIONS, 0), - baselineAligned = getLong(KEY_ALIGNED, 0), - leakCounts = LeakKind.entries.associateWith { kind -> - getLong("leak_${kind.id}", 0) - }, - ) + } else { + PlayerHistory() } + cachedHistory = loaded + return loaded + } - @SuppressLint("UseKtx") // KTX edit discards commit(), so durability failures cannot be reported. - fun SharedPreferences.writeHistory(history: PlayerHistory): Boolean { - val editor = edit() - .putInt(KEY_SCHEMA, history.schemaVersion) - .putString(KEY_PLAYER_NAME, history.playerName) - .putBoolean(KEY_COACH_ENABLED, history.coachEnabled) - .putLong(KEY_SESSIONS, history.sessionsStarted) - .putLong(KEY_HANDS, history.handsCompleted) - .putLong(KEY_DECISIONS, history.decisionsReviewed) - .putLong(KEY_ALIGNED, history.baselineAligned) - for (kind in LeakKind.entries) { - editor.putLong("leak_${kind.id}", history.leakCount(kind)) - } - return editor.commit() - } + private fun disablePersistence(message: String, error: RuntimeException?) { + persistenceAvailable = false + // Reporting must not turn a handled storage failure back into a crash. + runCatching { failureReporter(message, error) } } } + +class SharedPreferencesPlayerHistoryStore(context: Context) : PlayerHistoryStore by + ResilientPlayerHistoryStore( + persistence = SharedPreferencesHistoryPersistence( + context.getSharedPreferences(PREFERENCES_NAME, Context.MODE_PRIVATE), + ), + failureReporter = { message, error -> + if (error == null) Log.e(LOG_TAG, message) else Log.e(LOG_TAG, message, error) + }, + ) + +internal enum class HistorySchemaPlan { + FRESH, + READ_VERSION_1, + UNSUPPORTED, +} + +/** + * Explicit migration dispatch. Add a branch and migration before incrementing + * [PLAYER_HISTORY_SCHEMA_VERSION]; never reinterpret a non-additive schema. + */ +internal fun historySchemaPlan(storedVersion: Int): HistorySchemaPlan = when (storedVersion) { + 0 -> HistorySchemaPlan.FRESH + 1 -> HistorySchemaPlan.READ_VERSION_1 + else -> HistorySchemaPlan.UNSUPPORTED +} + +internal class UnsupportedPlayerHistorySchema(storedVersion: Int) : + IllegalStateException( + "player history schema $storedVersion is not supported by schema " + + PLAYER_HISTORY_SCHEMA_VERSION, + ) + +private class SharedPreferencesHistoryPersistence( + private val preferences: SharedPreferences, +) : PlayerHistoryPersistence { + override fun read(): PlayerHistory { + val storedSchema = if (preferences.contains(KEY_SCHEMA)) { + preferences.getInt(KEY_SCHEMA, 0) + } else { + 0 + } + return when (historySchemaPlan(storedSchema)) { + HistorySchemaPlan.FRESH -> PlayerHistory() + HistorySchemaPlan.READ_VERSION_1 -> preferences.readVersion1() + HistorySchemaPlan.UNSUPPORTED -> + throw UnsupportedPlayerHistorySchema(storedSchema) + } + } + + @SuppressLint("UseKtx") // KTX edit discards commit(), so failures cannot be reported. + override fun write(history: PlayerHistory): Boolean { + val editor = preferences.edit() + .putInt(KEY_SCHEMA, history.schemaVersion) + .putString(KEY_PLAYER_NAME, history.playerName) + .putBoolean(KEY_COACH_ENABLED, history.coachEnabled) + .putLong(KEY_SESSIONS, history.sessionsStarted) + .putLong(KEY_HANDS, history.handsCompleted) + .putLong(KEY_DECISIONS, history.decisionsReviewed) + .putLong(KEY_ALIGNED, history.baselineAligned) + for (kind in LeakKind.entries) { + editor.putLong("leak_${kind.id}", history.leakCount(kind)) + } + return editor.commit() + } + + private fun SharedPreferences.readVersion1(): PlayerHistory = PlayerHistory( + playerName = getString(KEY_PLAYER_NAME, "").orEmpty(), + coachEnabled = getBoolean(KEY_COACH_ENABLED, false), + sessionsStarted = getLong(KEY_SESSIONS, 0), + handsCompleted = getLong(KEY_HANDS, 0), + decisionsReviewed = getLong(KEY_DECISIONS, 0), + baselineAligned = getLong(KEY_ALIGNED, 0), + leakCounts = LeakKind.entries.associateWith { kind -> + getLong("leak_${kind.id}", 0) + }, + ) +} + +private const val PREFERENCES_NAME = "player_history" +private const val LOG_TAG = "PlayerHistory" +private const val KEY_SCHEMA = "schema_version" +private const val KEY_PLAYER_NAME = "player_name" +private const val KEY_COACH_ENABLED = "coach_enabled" +private const val KEY_SESSIONS = "sessions_started" +private const val KEY_HANDS = "hands_completed" +private const val KEY_DECISIONS = "decisions_reviewed" +private const val KEY_ALIGNED = "baseline_aligned" diff --git a/app/src/test/java/com/jsjdesigns/poker/PlayerHistoryStoreTest.kt b/app/src/test/java/com/jsjdesigns/poker/PlayerHistoryStoreTest.kt new file mode 100644 index 0000000..07fb32c --- /dev/null +++ b/app/src/test/java/com/jsjdesigns/poker/PlayerHistoryStoreTest.kt @@ -0,0 +1,122 @@ +package com.jsjdesigns.poker + +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class PlayerHistoryStoreTest { + + private class FakePersistence( + var stored: PlayerHistory = PlayerHistory(), + var readFailure: RuntimeException? = null, + var writeResult: Boolean = true, + var writeFailure: RuntimeException? = null, + ) : PlayerHistoryPersistence { + var reads = 0 + var writes = 0 + + override fun read(): PlayerHistory { + reads++ + readFailure?.let { throw it } + return stored + } + + override fun write(history: PlayerHistory): Boolean { + writes++ + writeFailure?.let { throw it } + if (writeResult) stored = history + return writeResult + } + } + + @Test + fun `schema dispatch has an explicit migration branch`() { + assertEquals(HistorySchemaPlan.FRESH, historySchemaPlan(0)) + assertEquals(HistorySchemaPlan.READ_VERSION_1, historySchemaPlan(1)) + assertEquals(HistorySchemaPlan.UNSUPPORTED, historySchemaPlan(2)) + assertEquals(HistorySchemaPlan.UNSUPPORTED, historySchemaPlan(99)) + assertEquals(HistorySchemaPlan.UNSUPPORTED, historySchemaPlan(-1)) + } + + @Test + fun `newer schema falls back in memory without overwrite or crash`() = runTest { + val persistence = FakePersistence( + readFailure = UnsupportedPlayerHistorySchema(99), + ) + val failures = mutableListOf() + val store = ResilientPlayerHistoryStore(persistence) { message, _ -> + failures += message + } + + assertEquals(PlayerHistory(), store.load()) + val currentSession = store.recordSession("Jay", coachEnabled = true) + + assertEquals("Jay", currentSession.playerName) + assertTrue(currentSession.coachEnabled) + assertEquals(1L, currentSession.sessionsStarted) + assertEquals("fallback remains useful in this process", currentSession, store.load()) + assertEquals(1, persistence.reads) + assertEquals("an older build must not overwrite newer data", 0, persistence.writes) + assertEquals(1, failures.size) + } + + @Test + fun `corrupt history also degrades to an in-memory session`() = runTest { + val persistence = FakePersistence( + readFailure = IllegalArgumentException("corrupt counter"), + ) + val store = ResilientPlayerHistoryStore(persistence) + + assertEquals(PlayerHistory(), store.load()) + assertEquals(1L, store.recordCompletedHand().handsCompleted) + assertEquals(0, persistence.writes) + } + + @Test + fun `failed commit disables further writes but keeps current counters`() = runTest { + val persistence = FakePersistence(writeResult = false) + val failures = mutableListOf() + val store = ResilientPlayerHistoryStore(persistence) { message, _ -> + failures += message + } + + assertEquals(1L, store.recordCompletedHand().handsCompleted) + assertEquals(2L, store.recordCompletedHand().handsCompleted) + + assertEquals("do not repeatedly hammer failed storage", 1, persistence.writes) + assertEquals(1, failures.size) + assertTrue("commit" in failures.single()) + } + + @Test + fun `write exception is contained like a false commit`() = runTest { + val persistence = FakePersistence( + writeFailure = IllegalStateException("disk unavailable"), + ) + val store = ResilientPlayerHistoryStore(persistence) + + assertEquals(1L, store.recordCompletedHand().handsCompleted) + assertEquals(2L, store.recordCompletedHand().handsCompleted) + assertEquals(1, persistence.writes) + } + + @Test + fun `concurrent hand updates cannot clobber one another`() = runTest { + val persistence = FakePersistence() + val store = ResilientPlayerHistoryStore(persistence) + + coroutineScope { + repeat(100) { + launch { store.recordCompletedHand() } + } + } + + assertEquals(100L, store.load().handsCompleted) + assertEquals(100, persistence.writes) + assertFalse(persistence.stored.coachEnabled) + } +}