From f05fc35e4c56287bde670d9d35982ae17a247244 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 17 Sep 2026 10:05:12 -0400 Subject: [PATCH 1/2] fix(chat): group a transcript's runs by sender id, not by resolved profile A group's incoming bubbles are cornered as one run until the member profiles land: First at the top, Middle through the rest, whoever actually sent them. isSameAuthorAs compared sender?.userId, and `sender` is filled from the profile map that ChatViewModel combines into the paging stream. That map starts null and arrives after the first page, so early on every member's bubble has a null sender and they all compare equal. bubblePositionOf and rowGapBelow both read that answer, so the run gets one set of corners and the tight same-sender gap. A member whose profile never resolves stays merged for good. ContentBubble now carries senderId straight off the message, independent of the lookup, and authorId prefers it. The profile is still what draws the name and the picture; it is no longer what decides who said it. --- .../app/messenger/internal/ChatViewModel.kt | 4 + .../shared/chat/models/ChatListItem.kt | 25 ++++-- .../shared/chat/ui/BubblePositionTest.kt | 88 +++++++++++++++++++ 3 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubblePositionTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 11ae602285..b5213963c4 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -507,6 +507,10 @@ internal class ChatViewModel @Inject constructor( capabilities = resolveCapabilities(message, policy), quote = quote, sender = sender, + // Independent of the profile lookup above: the runs have to break by + // author from the first frame, and the map that names the authors lands + // after the first page does. + senderId = message.senderId?.takeIf { !message.isFromSelf }, ) } }.insertSeparators { before: ChatListItem.ContentBubble?, after: ChatListItem.ContentBubble? -> diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt index b29a07660e..538149f428 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt @@ -2,6 +2,7 @@ package com.flipcash.shared.chat.models import com.flipcash.services.models.chat.MessageContent import com.flipcash.shared.chat.MessageCapability +import com.getcode.opencode.model.core.ID import kotlin.time.Instant enum class ReceiptStatus { SENDING, SENT, READ, FAILED } @@ -48,20 +49,34 @@ sealed interface ChatListItem { * message in a DM; set only for another member's message in a group. */ val sender: SenderIdentity? = null, + /** + * The id of whoever sent this, for every incoming message — set whether or not their + * profile has resolved, and whether or not the chat attributes its bubbles. + * + * [sender] cannot stand in for it: a group's profile map arrives after the first page does, + * so early on every member's bubble has a null [sender] and two members read as one author. + */ + val senderId: ID? = null, ) : ChatListItem { + /** + * Who this bubble is attributed to, for grouping. [senderId] is the answer whenever the + * pipeline supplies it; [sender] covers a caller that builds a bubble from a resolved + * profile alone. + */ + val authorId: ID? get() = senderId ?: sender?.userId + /** * Whether [other] was sent by the same person as this bubble. * * `isFromSelf` cannot answer this in a group, where every other member's message is - * incoming alike: two members' bubbles would read as one run. [sender] is what tells them - * apart, and it is null only for the viewer's own messages and for DMs — cases - * `isFromSelf` already decides. Two bubbles whose sender profiles have not resolved yet do - * read as one author until they do. + * incoming alike: two members' bubbles would read as one run. [authorId] is what tells them + * apart, and it is null only for the viewer's own messages — a case `isFromSelf` already + * decides — and for an incoming message whose sender the backend did not name. */ fun isSameAuthorAs(other: ContentBubble): Boolean = when { isFromSelf != other.isFromSelf -> false isFromSelf -> true - else -> sender?.userId == other.sender?.userId + else -> authorId == other.authorId } /** The body a Copy or an Edit acts on, or `null` for a bubble that carries no text. */ diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubblePositionTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubblePositionTest.kt new file mode 100644 index 0000000000..3634282beb --- /dev/null +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubblePositionTest.kt @@ -0,0 +1,88 @@ +package com.flipcash.shared.chat.ui + +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.models.SenderIdentity +import com.flipcash.shared.chat.models.SeparatorConfig +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * How a group's bubbles are cornered: each member's run is shaped on its own, First at the top + * through Last at the bottom, and a change of member restarts it. + * + * The list is held newest-first, as the paging snapshot holds it — index + 1 is the bubble drawn + * above, index - 1 the one drawn below. + */ +class BubblePositionTest { + + private val alice = + SenderIdentity(userId = listOf(1), displayName = "Alice", picture = null) + private val bob = SenderIdentity(userId = listOf(2), displayName = "Bob", picture = null) + private val start = Instant.fromEpochMilliseconds(1_700_000_000_000) + + private fun bubble( + id: Long, + secondsIn: Long, + sender: SenderIdentity? = null, + senderId: List? = null, + ) = ChatListItem.ContentBubble( + messageId = id, + contentIndex = 0, + content = MessageContent.Text("m$id"), + isFromSelf = false, + timestamp = start + secondsIn.seconds, + sender = sender, + senderId = senderId, + ) + + private fun positionsOf(items: List) = + items.mapIndexed { index, item -> + bubblePositionOf(index, item, items, SeparatorConfig.DayOnly) + } + + @Test + fun `a run is cornered on its own and restarts at a change of sender`() { + val items = listOf( + bubble(4, secondsIn = 30, sender = bob), + bubble(3, secondsIn = 20, sender = bob), + bubble(2, secondsIn = 10, sender = alice), + bubble(1, secondsIn = 0, sender = alice), + ) + + assertEquals( + listOf( + BubblePosition.Last, // bob, newest — bottom of his run + BubblePosition.First, // bob, top of his run + BubblePosition.Last, // alice, bottom of hers + BubblePosition.First, // alice, oldest + ), + positionsOf(items), + ) + } + + @Test + fun `two members read as two runs before their profiles resolve`() { + // What the first frame of a group transcript looks like: the profile map has not arrived, + // so no bubble carries a `sender` yet. Grouping them by the resolved profile merged every + // member into one run and cornered it as one. + val items = listOf( + bubble(4, secondsIn = 30, senderId = bob.userId), + bubble(3, secondsIn = 20, senderId = bob.userId), + bubble(2, secondsIn = 10, senderId = alice.userId), + bubble(1, secondsIn = 0, senderId = alice.userId), + ) + + assertEquals( + listOf( + BubblePosition.Last, + BubblePosition.First, + BubblePosition.Last, + BubblePosition.First, + ), + positionsOf(items), + ) + } +} From 359ce2fb11ebcf485cbfc41aa6e3792c15d51cb2 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 17 Sep 2026 10:05:18 -0400 Subject: [PATCH 2/2] fix(chat): move a group run's avatar to its last bubble MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picture sat on the same bubble as the name, at the top of the run. It now sits on the run's last message — the newest thing that person said, level with the bottom bubble — while the name stays on the first, so the two mark the run's ends instead of stacking. endsSenderRun is the mirror of startsSenderRun: `newer` is peek(index - 1) under reverseLayout, where `older` is peek(index + 1). Both key off authorId rather than the resolved sender, so a run holds its shape from the first frame instead of re-splitting when the profiles arrive. --- .../internal/screens/components/MessageRow.kt | 34 +++++++-- .../app/messenger/internal/SenderRunTest.kt | 76 +++++++++++++++---- 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt index d4e2062233..aab1ad74eb 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt @@ -222,6 +222,11 @@ internal fun MessageRow( // message has no item above it — a one-message transcript crashed here. older = if (index + 1 < messages.itemCount) messages.peek(index + 1) else null, ) + val runEnd = endsSenderRun( + current = item, + // index - 1 is the row drawn below — the newer message, under `reverseLayout`. + newer = if (index > 0) messages.peek(index - 1) else null, + ) Row( modifier = Modifier.fillMaxWidth(), verticalAlignment = Alignment.Bottom, @@ -235,7 +240,7 @@ internal fun MessageRow( // when the name arrived. if (showsSenderGutter && !item.isFromSelf) { Box(modifier = Modifier.requiredSize(CodeTheme.dimens.staticGrid.x6)) { - if (sender != null && runStart) { + if (sender != null && runEnd) { ContactAvatar( image = sender.picture, displayName = sender.displayName, @@ -419,17 +424,36 @@ internal fun rowGapBelow(item: ChatListItem, below: ChatListItem?, config: Separ } /** - * Whether [current] is the bubble that wears its sender's name and picture. + * Whether [current] is the bubble that wears its sender's name — the top of a run. * * `older` is the item drawn *above* [current] — under `reverseLayout` that is `peek(index + 1)`, - * the mirror of [bottomSpacingFor]'s `index - 1`. A run is labelled at its top, so the bubble that + * the mirror of [bottomSpacingFor]'s `index - 1`. A run is named at its top, so the bubble that * starts one is the one whose upper neighbour came from someone else. * + * Keyed off `authorId` rather than off the resolved `sender`, so the runs hold their shape from the + * first frame: a group's profile map arrives after the first page, and until it does every member's + * bubble has a null `sender`. + * * Pure so it can be tested without a `PagingData`: the messenger module has no `paging-testing` * dependency, and the peek belongs to the caller anyway. */ internal fun startsSenderRun(current: ChatListItem.ContentBubble, older: ChatListItem?): Boolean { - val sender = current.sender ?: return false + val author = current.authorId ?: return false val olderBubble = older as? ChatListItem.ContentBubble ?: return true - return olderBubble.sender?.userId != sender.userId + return olderBubble.authorId != author +} + +/** + * Whether [current] is the bubble that wears its sender's picture — the bottom of a run. + * + * The picture sits on the run's last message, next to the newest thing that person said, while the + * name stays on its first: the two mark the run's ends rather than stacking on one bubble. + * + * `newer` is the item drawn *below* [current] — `peek(index - 1)` under `reverseLayout`, the mirror + * of [startsSenderRun]'s `older`. + */ +internal fun endsSenderRun(current: ChatListItem.ContentBubble, newer: ChatListItem?): Boolean { + val author = current.authorId ?: return false + val newerBubble = newer as? ChatListItem.ContentBubble ?: return true + return newerBubble.authorId != author } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SenderRunTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SenderRunTest.kt index 4f1f5bc3d6..9beac0a69c 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SenderRunTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/SenderRunTest.kt @@ -1,5 +1,6 @@ package com.flipcash.app.messenger.internal +import com.flipcash.app.messenger.internal.screens.components.endsSenderRun import com.flipcash.app.messenger.internal.screens.components.startsSenderRun import com.flipcash.shared.chat.models.ChatListItem import com.flipcash.shared.chat.models.SenderIdentity @@ -10,26 +11,31 @@ import kotlin.test.assertTrue import kotlin.time.Instant /** - * Which bubble in a run wears the name and the picture. + * Which bubble in a run wears the name, and which wears the picture. * - * The list is `reverseLayout`, so index 0 is the newest message at the bottom and index + 1 is the - * message drawn *above*. A run therefore starts at its oldest message — the topmost one — which is - * the one whose upper neighbour is someone else, a date separator, or nothing. + * The list is `reverseLayout`, so index 0 is the newest message at the bottom, index + 1 is the + * message drawn *above* and index - 1 the one drawn *below*. A run therefore starts at its oldest + * message — the topmost one, which wears the name — and ends at its newest, the bottom one, which + * wears the picture. */ class SenderRunTest { private val alice = SenderIdentity(userId = listOf(1), displayName = "Alice", picture = null) private val bob = SenderIdentity(userId = listOf(2), displayName = "Bob", picture = null) - private fun bubble(sender: SenderIdentity?, isFromSelf: Boolean = false) = - ChatListItem.ContentBubble( - messageId = 1L, - contentIndex = 0, - content = MessageContent.Text("hi"), - isFromSelf = isFromSelf, - timestamp = Instant.fromEpochMilliseconds(1_000), - sender = sender, - ) + private fun bubble( + sender: SenderIdentity?, + isFromSelf: Boolean = false, + senderId: List? = null, + ) = ChatListItem.ContentBubble( + messageId = 1L, + contentIndex = 0, + content = MessageContent.Text("hi"), + isFromSelf = isFromSelf, + timestamp = Instant.fromEpochMilliseconds(1_000), + sender = sender, + senderId = senderId, + ) @Test fun `a bubble with no sender never starts a run`() { @@ -62,4 +68,48 @@ class SenderRunTest { val separator = ChatListItem.DateSeparator(Instant.fromEpochMilliseconds(1_000)) assertTrue(startsSenderRun(bubble(alice), older = separator)) } + + @Test + fun `an unresolved profile still breaks the run at its sender`() { + // A group's profile map lands after the first page, so `sender` is null on every member's + // bubble to begin with. The id is what has to tell them apart until then. + val unnamedAlice = bubble(sender = null, senderId = alice.userId) + val unnamedBob = bubble(sender = null, senderId = bob.userId) + assertTrue(startsSenderRun(unnamedAlice, older = unnamedBob)) + assertFalse(startsSenderRun(unnamedAlice, older = unnamedAlice)) + assertTrue(endsSenderRun(unnamedAlice, newer = unnamedBob)) + assertFalse(endsSenderRun(unnamedAlice, newer = unnamedAlice)) + } + + @Test + fun `a bubble with no sender never ends a run`() { + assertFalse(endsSenderRun(bubble(sender = null), newer = null)) + assertFalse(endsSenderRun(bubble(sender = null, isFromSelf = true), newer = bubble(alice))) + } + + @Test + fun `the newest loaded bubble ends a run`() { + assertTrue(endsSenderRun(bubble(alice), newer = null)) + } + + @Test + fun `a bubble over a different sender ends a run`() { + assertTrue(endsSenderRun(bubble(alice), newer = bubble(bob))) + } + + @Test + fun `a bubble over the same sender continues the run`() { + assertFalse(endsSenderRun(bubble(alice), newer = bubble(alice))) + } + + @Test + fun `a bubble over one of the viewer's own ends a run`() { + assertTrue(endsSenderRun(bubble(alice), newer = bubble(sender = null, isFromSelf = true))) + } + + @Test + fun `a bubble over a date separator ends a run`() { + val separator = ChatListItem.DateSeparator(Instant.fromEpochMilliseconds(1_000)) + assertTrue(endsSenderRun(bubble(alice), newer = separator)) + } }