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)