From aafa96224555fcc01c24245a028b5e10381bfd8a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 22:49:24 +0000 Subject: [PATCH] fix(polls): audit fixes across the tally, the card and the screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A draw now has no winner. winning() used maxByOrNull, which handed the green highlight and the check to whichever tied option happened to come first — on a 1-1 poll, an outright lie about the result. The feed card no longer sorts every voter on the UI thread to draw four avatars. filterTo ran during composition and again on every tally change, sorting an option's whole voter list so UserGallery could take(4). TallyResults now keeps its voters unsorted, materialises the ordered list lazily for the screens that list everyone, and offers topUsers(n) via a bounded insertion. Desktop's own gallery and its is-this-my-vote check go through the same paths instead of forcing the sort. Voter rows are keyed by pubkey rather than by User instance. The cache normally hands out one User per pubkey, but an eviction and re-create would leave two live instances for one person — and two rows sharing a LazyColumn key is a crash, not a cosmetic duplicate. The tally still counts them separately; this stops the crash. An empty poll that is still loading no longer says "No votes yet" and "Loading every vote..." at the same time. The results opt-in fires on a deep link. It read note.event inside an id-keyed LaunchedEffect, so when the poll event arrived after first composition the effect never ran again and the visit went unrecorded. Keyed on the observed note state now, via observeNote, which also carries the subscription the screen already needed. The backfill remembers. The ViewModel is rebuilt on every visit, so bouncing in and out of a poll re-walked every relay's whole history; a completed drain now stands in for the next five minutes, which the live subscription makes safe. Smaller: the ViewModel factory, loader and mapped Flow are remembered instead of rebuilt each recomposition; winning() is computed once per build rather than once per option; myVote falls out of the loop already running instead of a second scan; option bars grow from zero on first show; the vote column has a max width instead of a fixed one; and a no-HLL merge reports approximate when any relay said so, not just when the highest did. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AQorYBSv9oMvKa8hF1uGBv --- .../amethyst/ui/note/types/Poll.kt | 14 ++-- .../polls/results/PollResultsScreen.kt | 62 ++++++++------ .../polls/results/RelayPollResponseLoader.kt | 40 +++++++-- .../model/nip88Polls/PollResponsesCache.kt | 76 +++++++++++++++-- .../nip88Polls/PollResultsViewModel.kt | 51 ++++++++--- .../nip88Polls/PollResponsesCacheTest.kt | 84 +++++++++++++++++++ .../nip88Polls/PollResultsViewModelTest.kt | 43 ++++++++++ .../desktop/ui/note/DesktopPollCard.kt | 4 +- .../quartz/nip45Count/CountMerge.kt | 9 +- 9 files changed, 324 insertions(+), 59 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Poll.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Poll.kt index b189c487a4..260bbe24ad 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Poll.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/Poll.kt @@ -575,7 +575,7 @@ private fun RenderClosedItem( // The percentage alone never said how many people that was. Text( - text = tally.users.size.toString(), + text = tally.size.toString(), style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.placeholderText, modifier = Modifier.padding(end = 6.dp), @@ -612,23 +612,27 @@ fun measure100PercentWidthModifier(textStyle: TextStyle): Modifier { } } +/** Faces drawn before the rest collapse into a "+N" chip. */ +private const val GALLERY_FACES = 4 + @Composable fun UserGallery( tally: TallyResults, galleryUser: @Composable RowScope.(user: User) -> Unit, ) { - if (tally.users.isNotEmpty()) { + if (tally.size > 0) { + val shown = tally.topUsers(GALLERY_FACES) Row( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy((-10).dp), ) { - tally.users.take(4).forEach { + shown.forEach { key(it.pubkeyHex) { galleryUser(it) } } - if (tally.users.size > 4) { + if (tally.size > shown.size) { Box( contentAlignment = Alignment.Center, modifier = @@ -638,7 +642,7 @@ fun UserGallery( .background(MaterialTheme.colorScheme.secondaryContainer), ) { Text( - text = "+" + showCount(tally.users.size - 4), + text = "+" + showCount(tally.size - shown.size), fontSize = 10.sp, color = MaterialTheme.colorScheme.onSurface, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/PollResultsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/PollResultsScreen.kt index f491491f77..1f30052485 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/PollResultsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/PollResultsScreen.kt @@ -41,6 +41,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width +import androidx.compose.foundation.layout.widthIn import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.foundation.rememberScrollState @@ -54,7 +55,9 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.key +import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -76,7 +79,7 @@ import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollOptionResult import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollResultsUiState import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollResultsViewModel import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollVoterRow -import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.EventFinderFilterAssemblerSubscription +import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNote import com.vitorpamplona.amethyst.ui.components.LoadNote import com.vitorpamplona.amethyst.ui.navigation.navs.INav import com.vitorpamplona.amethyst.ui.navigation.routes.routeFor @@ -103,7 +106,7 @@ import com.vitorpamplona.quartz.nip88Polls.poll.tags.PollType import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.flow.map -private val VoteColumnWidth = 116.dp +private val VoteColumnMaxWidth = 140.dp /** * The full results of a NIP-88 poll: how many votes each option got, and who voted for what. @@ -141,31 +144,34 @@ private fun PollResults( ) { val account = accountViewModel.account - // Keeps a REQ open for this poll while the screen is on top. Without it the only votes ever - // shown are the ones the one-shot backfill happened to catch: a vote cast while you are reading - // would never arrive, because the feed card that used to hold this subscription is disposed - // behind us. It also loads the kind-1068 event itself when we arrived by deep link. - EventFinderFilterAssemblerSubscription(note, accountViewModel) + // Subscribes for this poll while the screen is on top *and* observes what arrives. Without the + // subscription the only votes ever shown are the ones the one-shot backfill happened to catch — + // a vote cast while you are reading would never appear, because the feed card that used to + // carry it is disposed behind us. It also loads the kind-1068 event on a deep link. + val noteState by observeNote(note, accountViewModel) - // Opening this screen is the opt-in, exactly like the card's "View results" link — so the feed - // card stops hiding the tally behind a tap once you have been here. - LaunchedEffect(note.idHex) { + // Opening this screen is the opt-in, exactly like the card's "View results" link. Keyed on the + // note state rather than the id: arriving by deep link, the poll event lands *after* the first + // composition, and an id-keyed effect would never run again to record it. + LaunchedEffect(noteState) { (note.event as? PollEvent)?.let { accountViewModel.markPollResultsViewed(it.id, it.endsAt()) } } - val viewModel: PollResultsViewModel = - viewModel( - key = "PollResults-${note.idHex}", - factory = - PollResultsViewModel.Factory( - pollNote = note, - forKey = account.pubKey, - isHidden = { account.isHidden(it) }, - follows = account.allFollows.flow.map { it.authors }, - hiddenChanges = account.hiddenUsers.flow, - loader = RelayPollResponseLoader(account.client, account.cache, note), - ), - ) + // Remembered: viewModel() only consults the factory once, but building it inline allocated a + // loader, two lambdas and a mapped Flow on every recomposition. + val factory = + remember(note, account) { + PollResultsViewModel.Factory( + pollNote = note, + forKey = account.pubKey, + isHidden = { account.isHidden(it) }, + follows = account.allFollows.flow.map { it.authors }, + hiddenChanges = account.hiddenUsers.flow, + loader = RelayPollResponseLoader(account.client, account.cache, note), + ) + } + + val viewModel: PollResultsViewModel = viewModel(key = "PollResults-${note.idHex}", factory = factory) val state by viewModel.uiState.collectAsStateWithLifecycle() val selected by viewModel.selectedOption.collectAsStateWithLifecycle() @@ -414,8 +420,12 @@ private fun OptionBar( ) // Same 800ms tween the feed card uses, so a vote cast there and read here moves identically. + // animateFloatAsState starts *at* its target, so without stepping off zero the first frame the + // bars would simply appear at full length and only ever animate on later changes. + var target by remember(option.code) { mutableFloatStateOf(0f) } + LaunchedEffect(option.code, option.percent) { target = option.percent } val progress by animateFloatAsState( - targetValue = option.percent, + targetValue = target, animationSpec = tween(durationMillis = 800), label = "pollOptionBar", ) @@ -604,7 +614,7 @@ private fun VoterRow( private fun VoteChoice(voter: PollVoterRow) { Column( horizontalAlignment = Alignment.End, - modifier = Modifier.width(VoteColumnWidth), + modifier = Modifier.widthIn(max = VoteColumnMaxWidth), ) { Text( text = voter.labels.joinToString(", "), @@ -633,7 +643,7 @@ private fun ResultsFooter(state: PollResultsUiState) { } } - if (state.totalVoters == 0) { + if (state.totalVoters == 0 && !state.isBackfilling) { Box(Modifier.fillMaxWidth().padding(32.dp), contentAlignment = Alignment.Center) { Text( text = stringRes(R.string.poll_results_no_votes), diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/RelayPollResponseLoader.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/RelayPollResponseLoader.kt index d24599e1d7..96d2a545e3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/RelayPollResponseLoader.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/polls/results/RelayPollResponseLoader.kt @@ -24,6 +24,7 @@ import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollLoadReport import com.vitorpamplona.amethyst.commons.viewmodels.nip88Polls.PollResponseLoader import com.vitorpamplona.amethyst.model.LocalCache +import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.count import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.fetchAllPagesFromPool @@ -32,6 +33,8 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip45Count.mergeCountResults import com.vitorpamplona.quartz.nip88Polls.poll.PollEvent import com.vitorpamplona.quartz.nip88Polls.response.PollResponseEvent +import com.vitorpamplona.quartz.utils.TimeUtils +import java.util.concurrent.ConcurrentHashMap /** * Drains a poll's responses past the live subscription's cap, then asks the relays how many there @@ -55,9 +58,28 @@ class RelayPollResponseLoader( companion object { /** Per-relay idle window for both the drain and the COUNT. */ const val TIMEOUT_MS = 20_000L + + /** + * How long a completed drain stands in for the next one. + * + * The ViewModel — and therefore this loader — is rebuilt on every visit, so bouncing in and + * out of a poll would otherwise re-walk every relay's whole history each time. The live + * subscription keeps the tally current in between, which is what makes skipping safe. + */ + const val REDRAIN_AFTER_SECONDS = 5 * 60L + + // Process-wide, because the point is to outlive the screen. Small and self-limiting: one + // entry per poll actually opened. + private val recentDrains = ConcurrentHashMap>() + + fun forgetDrains() = recentDrains.clear() } override suspend fun load(poll: PollEvent): PollLoadReport { + recentDrains[poll.id]?.let { (drainedAt, report) -> + if (TimeUtils.now() - drainedAt < REDRAIN_AFTER_SECONDS) return report + } + val relays = responseRelays(poll) if (relays.isEmpty()) return PollLoadReport(null, approximate = false, relaysAsked = 0, relaysAnswered = 0) @@ -80,13 +102,17 @@ class RelayPollResponseLoader( // Combination rules (never a sum) live in quartz — see mergeCountResults. val merged = mergeCountResults(results.values) - return PollLoadReport( - reported = merged?.count, - // An estimate is approximate; so is a figure from only some of the relays we asked. - approximate = (merged?.approximate ?: false) || results.size < relays.size, - relaysAsked = relays.size, - relaysAnswered = results.size, - ) + val report = + PollLoadReport( + reported = merged?.count, + // An estimate is approximate; so is a figure from only some of the relays we asked. + approximate = (merged?.approximate ?: false) || results.size < relays.size, + relaysAsked = relays.size, + relaysAnswered = results.size, + ) + + recentDrains[poll.id] = TimeUtils.now() to report + return report } /** diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCache.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCache.kt index c36ee8840b..cc7bd071ef 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCache.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCache.kt @@ -83,7 +83,32 @@ class PollResponsesCache : UserDependencies { /** Voters whose response cast no valid option code at all, excluded from [tally]. */ val ignoredVotes: Int get() = votes.size - countedVoters.size - fun winning() = tally.maxByOrNull { it.value.size }?.key + /** + * The single option with the most voters, or null when nothing leads. + * + * A draw has no winner. `maxByOrNull` would hand the crown — the green highlight and + * the check — to whichever tied option happened to come first, which on a 1-1 poll is + * an outright lie about the result. + */ + fun winning(): String? { + var best: String? = null + var bestCount = 0 + var drawn = false + + tally.forEach { (code, voters) -> + when { + voters.size > bestCount -> { + best = code + bestCount = voters.size + drawn = false + } + + voters.size == bestCount -> drawn = true + } + } + + return if (drawn || bestCount == 0) null else best + } /** * Distinct people whose vote counts. This is the denominator for every percentage: on a @@ -210,7 +235,7 @@ class PollResponsesCache : UserDependencies { ): TallyResults { val comparator = compareByDescending { it.pubkeyHex == forKey }.thenByDescending { it.pubkeyHex in priority }.thenBy { it.pubkeyHex } - val usersThatVotedForThisOption = tally[code] ?: emptyList() + val usersThatVotedForThisOption = tally[code] ?: emptySet() val voters = totalVoters() @@ -224,9 +249,9 @@ class PollResponsesCache : UserDependencies { 0f } - val sortedUsers = usersThatVotedForThisOption.sortedWith(comparator) - - return TallyResults(sortedUsers, percent, code == winning()) + // Deliberately not sorted here: this runs on the UI thread for every visible poll card, and + // the feed only ever draws four avatars. TallyResults sorts on demand. + return TallyResults(usersThatVotedForThisOption, comparator, percent, code == winning()) } fun currentTally( @@ -251,9 +276,46 @@ class PollResponsesCache : UserDependencies { fun hasPubKeyVotedFlow(user: User): Flow = responses.map { it.countedVoters.contains(user) }.distinctUntilChanged() } +/** + * One option's share of a poll, with its voters kept unsorted until somebody needs them ordered. + * + * The feed card builds one of these per option, on the UI thread, every time the tally changes — + * and then draws four avatars. Sorting every voter of a busy poll to pick four is the kind of work + * that only shows up as dropped frames while scrolling, so [users] is lazy and [topUsers] never + * sorts the tail at all. + */ @Stable class TallyResults( - val users: List = emptyList(), + private val voters: Collection = emptyList(), + private val order: Comparator = compareBy { it.pubkeyHex }, val percent: Float = 0.0f, val isWinning: Boolean = false, -) +) { + val size get() = voters.size + + /** Whether this option holds a vote from [pubkeyHex], without materialising the ordered list. */ + fun contains(pubkeyHex: HexKey) = voters.any { it.pubkeyHex == pubkeyHex } + + /** Every voter, in order. Materialised once, and only for screens that list them all. */ + val users: List by lazy(LazyThreadSafetyMode.PUBLICATION) { voters.sortedWith(order) } + + /** + * The first [n] voters in order, chosen by a bounded insertion rather than a full sort — O(m·n) + * with n fixed at a handful, against O(m log m) to then throw all but [n] away. + */ + fun topUsers(n: Int): List { + if (n <= 0 || voters.isEmpty()) return emptyList() + if (voters.size <= n) return users + + val top = ArrayList(n + 1) + voters.forEach { user -> + val found = top.binarySearch(user, order) + val at = if (found < 0) -found - 1 else found + if (at < n) { + top.add(at, user) + if (top.size > n) top.removeAt(n) + } + } + return top + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModel.kt index 0cd76de132..a9da139939 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModel.kt @@ -202,6 +202,8 @@ class PollResultsViewModel( .thenBy { it.pubkeyHex } val totalVoters = tally.totalVoters() + // Once, not once per option: winning() walks every bucket. + val winner = tally.winning() val optionResults = codesInOrder.map { code -> @@ -211,24 +213,32 @@ class PollResultsViewModel( label = labels[code] ?: code, voters = voters.size, percent = if (totalVoters > 0) voters.size.toFloat() / totalVoters else 0f, - isWinning = voters.isNotEmpty() && code == tally.winning(), - topVoters = voters.sortedWith(byRelevance).take(AVATAR_STACK), + isWinning = code == winner, + topVoters = topByRelevance(voters, byRelevance, AVATAR_STACK), ) } // Invert the tally once: voter -> the codes they picked, in poll order. - val picks = mutableMapOf>() + // + // Keyed by pubkey rather than by User instance. The cache normally hands out one User per + // pubkey, but an eviction and re-create would leave two live instances for one person — + // and two rows sharing a key is a hard crash in LazyColumn, not a cosmetic duplicate. + val picks = LinkedHashMap>>() codesInOrder.forEach { code -> tally.tally[code]?.forEach { user -> - picks.getOrPut(user) { mutableListOf() }.add(code) + picks.getOrPut(user.pubkeyHex) { user to mutableListOf() }.second.add(code) } } var hidden = 0 + var myPicks: List = emptyList() val rows = picks - .mapNotNull { (user, codes) -> - if (isHidden(user.pubkeyHex)) { + .mapNotNull { (pubkey, entry) -> + val (user, codes) = entry + if (pubkey == forKey) myPicks = codes + + if (isHidden(pubkey)) { hidden++ return@mapNotNull null } @@ -250,11 +260,7 @@ class PollResultsViewModel( ignoredVotes = tally.ignoredVotes, lateVotes = tally.lateVotes, hiddenVoters = hidden, - myVote = - picks.entries - .firstOrNull { it.key.pubkeyHex == forKey } - ?.value - .orEmpty(), + myVote = myPicks, type = event?.pollType() ?: PollType.SINGLE_CHOICE, endsAt = event?.endsAt(), loadedResponses = tally.allResponses.size, @@ -267,6 +273,29 @@ class PollResultsViewModel( ) } + /** + * The [n] highest-ranked voters without sorting the rest — the stack shows a handful and a + * "+N" chip, so ordering the tail is pure waste on a poll with thousands of them. + */ + private fun topByRelevance( + voters: Collection, + order: Comparator, + n: Int, + ): List { + if (voters.size <= n) return voters.sortedWith(order) + + val top = ArrayList(n + 1) + voters.forEach { user -> + val found = top.binarySearch(user, order) + val at = if (found < 0) -found - 1 else found + if (at < n) { + top.add(at, user) + if (top.size > n) top.removeAt(n) + } + } + return top + } + class Factory( private val pollNote: Note, private val forKey: HexKey, diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCacheTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCacheTest.kt index 248e088fb3..15e35f335e 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCacheTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/nip88Polls/PollResponsesCacheTest.kt @@ -28,6 +28,7 @@ import com.vitorpamplona.quartz.nip88Polls.response.PollResponseEvent import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -411,6 +412,89 @@ class PollResponsesCacheTest { assertSame(before, cache.responses.value) } + @Test + fun aDrawHasNoWinner() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + + cache.addResponse(responseNote("1".repeat(64), "b".repeat(64), option = "yes", createdAt = 10)) + cache.addResponse(responseNote("2".repeat(64), "c".repeat(64), option = "no", createdAt = 10)) + + // Crowning whichever option came first would be an outright lie about a 1-1 result. + assertNull(cache.responses.value.winning()) + assertFalse(cache.currentTally("yes", "0".repeat(64), emptySet()).isWinning) + assertFalse(cache.currentTally("no", "0".repeat(64), emptySet()).isWinning) + } + + @Test + fun aClearLeadStillWins() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + + cache.addResponse(responseNote("1".repeat(64), "b".repeat(64), option = "yes", createdAt = 10)) + cache.addResponse(responseNote("2".repeat(64), "c".repeat(64), option = "yes", createdAt = 10)) + cache.addResponse(responseNote("3".repeat(64), "d".repeat(64), option = "no", createdAt = 10)) + + assertEquals("yes", cache.responses.value.winning()) + } + + @Test + fun anEmptyPollHasNoWinner() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + + assertNull(cache.responses.value.winning()) + } + + @Test + fun topUsersPicksTheHighestRankedWithoutOrderingTheRest() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + val me = "f".repeat(64) + val followed = "e".repeat(64) + + // Ten voters, two of whom should outrank the rest. + cache.addResponse(responseNote("1".repeat(64), me, option = "yes", createdAt = 10)) + cache.addResponse(responseNote("2".repeat(64), followed, option = "yes", createdAt = 10)) + (0 until 8).forEach { i -> + cache.addResponse(responseNote("$i".repeat(64).take(64).padEnd(64, 'a'), "${i}b".repeat(32), option = "yes", createdAt = 10)) + } + + val tally = cache.currentTally("yes", me, priorityAccounts = setOf(followed)) + + assertEquals(10, tally.size) + val top = tally.topUsers(4) + assertEquals(4, top.size) + assertEquals(me, top[0].pubkeyHex) + assertEquals(followed, top[1].pubkeyHex) + // Same answer the full ordering gives, just without paying for the tail. + assertEquals(tally.users.take(4).map { it.pubkeyHex }, top.map { it.pubkeyHex }) + } + + @Test + fun topUsersHandlesFewerVotersThanAsked() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + cache.addResponse(responseNote("1".repeat(64), "b".repeat(64), option = "yes", createdAt = 10)) + + val tally = cache.currentTally("yes", "0".repeat(64), emptySet()) + assertEquals(1, tally.topUsers(4).size) + assertEquals(0, tally.topUsers(0).size) + assertEquals(0, cache.currentTally("no", "0".repeat(64), emptySet()).topUsers(4).size) + } + + @Test + fun containsFindsAVoterWithoutOrdering() { + val cache = PollResponsesCache() + cache.updatePolicy(policy(PollType.SINGLE_CHOICE)) + val voter = "b".repeat(64) + cache.addResponse(responseNote("1".repeat(64), voter, option = "yes", createdAt = 10)) + + val tally = cache.currentTally("yes", "0".repeat(64), emptySet()) + assertTrue(tally.contains(voter)) + assertFalse(tally.contains("c".repeat(64))) + } + @Test fun voterWhoseVoteWasIgnoredCanStillVote() { val cache = PollResponsesCache() diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModelTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModelTest.kt index 3292ee828d..92f9835a2f 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModelTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/nip88Polls/PollResultsViewModelTest.kt @@ -254,6 +254,49 @@ class PollResultsViewModelTest { assertEquals(followed, red.topVoters[1].pubkeyHex) } + @Test + fun twoUserInstancesForOnePersonYieldOneRow() = + runTest { + val note = pollNote() + val pubkey = "9".repeat(64) + + // The cache normally hands out one User per pubkey, but an eviction and re-create can + // leave two live instances. Two rows sharing a LazyColumn key is a crash, not a + // cosmetic duplicate — so the row list has to collapse them. + listOf("1", "2").forEachIndexed { i, id -> + val event = + PollResponseEvent( + id = id.repeat(64), + pubKey = pubkey, + createdAt = 150L + i, + tags = arrayOf(arrayOf("e", pollId), arrayOf("response", "red")), + content = "", + sig = "0".repeat(128), + ) + val responseNote = Note(event.id) + // A fresh User object each time, deliberately bypassing the test's user cache. + responseNote.loadEvent(event, User(pubkey) { addr -> Note(addr.toValue()) }, emptyList()) + note.pollState().addResponse(responseNote) + } + + val state = stateOf(viewModel(note)) + + assertEquals(1, state.voters.count { it.user.pubkeyHex == pubkey }) + assertEquals(state.voters.size, state.voters.distinctBy { it.user.pubkeyHex }.size) + } + + @Test + fun aDrawLeavesEveryOptionUncrowned() = + runTest { + val note = pollNote() + vote(note, "1".repeat(64), me, listOf("red")) + vote(note, "2".repeat(64), followed, listOf("blue")) + + val state = stateOf(viewModel(note)) + + assertTrue(state.options.none { it.isWinning }, "a 1-1 poll has no winner to highlight") + } + @Test fun myVoteIsReportedForHighlightingTheOption() = runTest { diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/note/DesktopPollCard.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/note/DesktopPollCard.kt index 6fa5219d06..7a9dc76585 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/note/DesktopPollCard.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/ui/note/DesktopPollCard.kt @@ -448,7 +448,7 @@ private fun PollResultRow( animated.animateTo(tally.percent) } - val isMyVote = forKey.isNotEmpty() && tally.users.any { it.pubkeyHex == forKey } + val isMyVote = forKey.isNotEmpty() && tally.contains(forKey) val winning = tally.isWinning val barColor = if (winning) MaterialTheme.colorScheme.tertiary else MaterialTheme.colorScheme.primary // Border marks YOUR choice (primary); the winner is conveyed by the bar fill color. @@ -508,7 +508,7 @@ private fun PollResultRow( } Spacer(Modifier.width(12.dp)) Row(verticalAlignment = Alignment.CenterVertically) { - VoterGallery(tally.users, forKey) + VoterGallery(tally.topUsers(4), forKey) Spacer(Modifier.width(8.dp)) Text( text = "${(tally.percent * 100).toInt()}%", diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip45Count/CountMerge.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip45Count/CountMerge.kt index 076f893589..e2e78a31df 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip45Count/CountMerge.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip45Count/CountMerge.kt @@ -50,7 +50,14 @@ fun mergeCountResults(results: Collection): CountResult? { val hlls = results.mapNotNull { it.hll } if (hlls.isEmpty()) { - return results.maxByOrNull { it.count } + val best = results.maxByOrNull { it.count } ?: return null + // Approximation is a property of the batch, not of the winner: one relay admitting its + // count is an estimate makes the combined figure an estimate too, whoever reported highest. + return if (best.approximate || results.none { it.approximate }) { + best + } else { + CountResult(count = best.count, approximate = true, hll = null) + } } val merged = HyperLogLog.merge(hlls)