fix(chat): group-chat sender runs — avatar on the last bubble, runs broken by sender id

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? ->

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
 }

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<Byte>(1), displayName = "Alice", picture = null)
     private val bob = SenderIdentity(userId = listOf<Byte>(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<Byte>? = 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))
+    }
 }

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. */

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<Byte>(1), displayName = "Alice", picture = null)
+    private val bob = SenderIdentity(userId = listOf<Byte>(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<Byte>? = 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<ChatListItem.ContentBubble>) =
+        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),
+        )
+    }
+}