fix(chat): separate sender runs in a group by the author, not by isFromSelf

apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt

@@ -36,6 +36,7 @@ import androidx.paging.compose.LazyPagingItems
 import androidx.paging.compose.itemKey
 import com.flipcash.app.messenger.internal.ChatViewModel
 import com.flipcash.app.messenger.internal.screens.ChatAnimations
+import com.flipcash.services.models.chat.ChatType
 import com.flipcash.services.models.chat.MessagePointer
 import com.flipcash.shared.chat.models.ChatAction
 import com.flipcash.shared.chat.models.ChatActionHandler
@@ -69,6 +70,9 @@ internal fun MessageList(
     val keyboard = rememberKeyboardController()
     val listState = rememberLazyListState()
     val vibrator = LocalVibrator.current
+    // Whether messages are attributed at all, which is a property of the chat and not of any one
+    // row: a DM needs no avatar column, a group reserves one on every incoming row.
+    val isGroup = state.chatType == ChatType.GROUP
 
     CompositionLocalProvider(LocalChatActionHandler provides onAction) {
         HandleMessageReads(listState, messages)
@@ -280,6 +284,7 @@ internal fun MessageList(
                     selecting = selecting,
                     focused = focused,
                     animateInsertion = animateInsertion,
+                    showsSenderGutter = isGroup,
                     attention = if (bubble != null && bubble.messageId == attentionId) {
                         readAttention
                     } else {

apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt

@@ -67,7 +67,9 @@ import com.getcode.util.vibration.LocalVibrator
  * the row below — and takes the rest as flags, because they are decided across the whole list:
  * [selecting] is true for every row while the backdrop is up, [focused] for the single row it leaves
  * sharp, [attention] carries the flash the list points at a jumped-to message, and
- * [animateInsertion] is granted once per message and never again.
+ * [animateInsertion] is granted once per message and never again, and [showsSenderGutter] says
+ * whether incoming rows reserve the avatar column — true for a group, where every message is
+ * attributed, and false for a DM, where none is.
  */
 @OptIn(ExperimentalFoundationApi::class)
 @Composable
@@ -80,6 +82,7 @@ internal fun MessageRow(
     selecting: Boolean,
     focused: Boolean,
     animateInsertion: Boolean,
+    showsSenderGutter: Boolean,
     attention: () -> Float = { 0f },
 ) {
     val onAction = LocalChatActionHandler.current
@@ -224,12 +227,15 @@ internal fun MessageRow(
                     verticalAlignment = Alignment.Bottom,
                     horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x1),
                 ) {
-                    // The gutter is reserved on every row of a sender run, not only the labelled
-                    // one, so the bubbles above and below line up on the same left edge instead of
-                    // stepping in and out as the run starts.
-                    if (sender != null) {
+                    // The gutter is reserved on every incoming row of a group, not only the
+                    // labelled ones, so bubbles line up on the same left edge instead of stepping
+                    // in and out as runs start. Keyed off the transcript rather than off this row's
+                    // `sender`, which is also null while a member's profile is still resolving — a
+                    // row that reserved no gutter would sit out at the inset and then jump inward
+                    // when the name arrived.
+                    if (showsSenderGutter && !item.isFromSelf) {
                         Box(modifier = Modifier.requiredSize(CodeTheme.dimens.staticGrid.x6)) {
-                            if (runStart) {
+                            if (sender != null && runStart) {
                                 ContactAvatar(
                                     image = sender.picture,
                                     displayName = sender.displayName,
@@ -368,27 +374,47 @@ private fun bottomSpacingFor(
     messages: LazyPagingItems<ChatListItem>,
     config: SeparatorConfig,
 ): Dp {
-    val tight = CodeTheme.dimens.grid.x1
-    val normal = CodeTheme.dimens.grid.x2
-    val wide = CodeTheme.dimens.grid.x3
     // index-1 is the item below (newer) in reverseLayout
-    val itemBelow = (if (index > 0) messages.peek(index - 1) else null) ?: return tight
+    val gap = rowGapBelow(item, if (index > 0) messages.peek(index - 1) else null, config)
+    return when (gap) {
+        RowGap.Tight -> CodeTheme.dimens.grid.x1
+        RowGap.Normal -> CodeTheme.dimens.grid.x2
+        RowGap.Wide -> CodeTheme.dimens.grid.x3
+    }
+}
+
+/** The three gaps the transcript puts between rows, resolved to `grid.x1`/`x2`/`x3`. */
+internal enum class RowGap { Tight, Normal, Wide }
+
+/**
+ * The gap under [item], where [below] is the row drawn beneath it — the newer message, `peek(index
+ * - 1)` under `reverseLayout`.
+ *
+ * A change of author opens the widest gap, because that is where the next run's name and picture
+ * go. `isFromSelf` alone used to stand in for the author, which is right in a DM and wrong in a
+ * group: two members' messages are both incoming, so a whole conversation between them ran at the
+ * tight same-sender gap.
+ *
+ * Pure so it can be tested without a `PagingData`, like [startsSenderRun]: the messenger module has
+ * no `paging-testing` dependency, and the peek belongs to the caller.
+ */
+internal fun rowGapBelow(item: ChatListItem, below: ChatListItem?, config: SeparatorConfig): RowGap {
+    below ?: return RowGap.Tight
 
     // Separator adjacent → normal gap
-    if (item is ChatListItem.DateSeparator || itemBelow is ChatListItem.DateSeparator) {
-        return normal
+    if (item is ChatListItem.DateSeparator || below is ChatListItem.DateSeparator) {
+        return RowGap.Normal
     }
 
-    val current = item as? ChatListItem.ContentBubble ?: return tight
-    val below = itemBelow as? ChatListItem.ContentBubble ?: return tight
+    val current = item as? ChatListItem.ContentBubble ?: return RowGap.Tight
+    val newer = below as? ChatListItem.ContentBubble ?: return RowGap.Tight
 
     return when {
-        // Different sender → wide
-        current.isFromSelf != below.isFromSelf -> wide
+        !current.isSameAuthorAs(newer) -> RowGap.Wide
         // Same sender, outside grouping window → normal
-        !config.isGrouped(current.timestamp, below.timestamp) -> normal
+        !config.isGrouped(current.timestamp, newer.timestamp) -> RowGap.Normal
         // Same sender, close together → tight
-        else -> tight
+        else -> RowGap.Tight
     }
 }
 

apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt

@@ -0,0 +1,154 @@
+package com.flipcash.app.messenger.internal
+
+import android.graphics.Bitmap
+import android.graphics.Canvas
+import android.view.View
+import androidx.activity.ComponentActivity
+import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.width
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.test.junit4.createAndroidComposeRule
+import androidx.compose.ui.unit.dp
+import androidx.paging.PagingData
+import androidx.paging.compose.LazyPagingItems
+import androidx.paging.compose.collectAsLazyPagingItems
+import com.flipcash.app.messenger.internal.screens.components.MessageRow
+import com.flipcash.app.theme.FlipcashThemeWrapper
+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 kotlinx.coroutines.flow.flowOf
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.Config
+import org.robolectric.annotation.GraphicsMode
+import java.io.File
+import kotlin.time.Duration.Companion.seconds
+import kotlin.time.Instant
+
+/**
+ * Renders a group transcript to a PNG in `build/screenshots/` so the run spacing and the avatar
+ * gutter can be eyeballed without an emulator. Not an assertion test.
+ *
+ * Rows are drawn in a plain `Column` rather than the real `LazyColumn`, oldest first, because the
+ * transcript's list is `reverseLayout` and index 0 is the newest message. What the rows compute
+ * from their neighbours — the gap below, whether they start a run — comes off the paging snapshot
+ * either way.
+ *
+ * Same mechanics as [GroupChromeScreenshotTest]: pump a fixed number of frames and draw the Android
+ * view directly, so a composable that keeps scheduling frames can't hang `captureToImage()`.
+ */
+@RunWith(RobolectricTestRunner::class)
+@GraphicsMode(GraphicsMode.Mode.NATIVE)
+@Config(sdk = [34], qualifiers = "w400dp-h1100dp-xhdpi")
+class GroupTranscriptScreenshotTest {
+
+    @get:Rule
+    val composeRule = createAndroidComposeRule<ComponentActivity>()
+
+    private val noah =
+        SenderIdentity(userId = listOf<Byte>(1), displayName = "Noah Parker", picture = null)
+    private val chloe =
+        SenderIdentity(userId = listOf<Byte>(2), displayName = "Chloe Anderson", picture = null)
+
+    private val start = Instant.fromEpochMilliseconds(1_700_000_000_000)
+
+    private fun bubble(
+        id: Long,
+        text: String,
+        sender: SenderIdentity?,
+        isFromSelf: Boolean = false,
+        secondsIn: Long = 0,
+    ) = ChatListItem.ContentBubble(
+        messageId = id,
+        contentIndex = 0,
+        content = MessageContent.Text(text),
+        isFromSelf = isFromSelf,
+        timestamp = start + secondsIn.seconds,
+        sender = sender,
+    )
+
+    @Test
+    fun rendersGroupTranscript() {
+        // Newest first, as the paging snapshot holds them.
+        val items = listOf(
+            bubble(6, "Fine, Saturday then", sender = null, isFromSelf = true, secondsIn = 50),
+            bubble(5, "Watch the tape", chloe, secondsIn = 40),
+            bubble(4, "What are you talking about. Did you see him last night?", chloe, secondsIn = 30),
+            bubble(3, "Not even close", noah, secondsIn = 20),
+            bubble(2, "I disagree. He isn't the best at all", noah, secondsIn = 10),
+            ChatListItem.DateSeparator(start),
+        )
+
+        composeRule.setContent {
+            FlipcashThemeWrapper().Wrap {
+                val messages = flowOf(PagingData.from(items)).collectAsLazyPagingItems()
+                Transcript(messages)
+            }
+        }
+        repeat(20) { composeRule.mainClock.advanceTimeByFrame() }
+
+        capture("group_transcript.png")
+    }
+
+    @Composable
+    private fun Transcript(messages: LazyPagingItems<ChatListItem>) {
+        Column(modifier = Modifier.width(360.dp).padding(16.dp)) {
+            // Oldest first: the list this stands in for is reverseLayout.
+            for (index in messages.itemCount - 1 downTo 0) {
+                val item = messages[index] ?: continue
+                MessageRow(
+                    index = index,
+                    item = item,
+                    messages = messages,
+                    separatorConfig = SeparatorConfig.DayOnly,
+                    otherReadPointer = null,
+                    selecting = false,
+                    focused = true,
+                    animateInsertion = false,
+                    showsSenderGutter = true,
+                )
+            }
+        }
+    }
+
+    private fun capture(name: String) {
+        val root: View = composeRule.activity.findViewById(android.R.id.content)
+        val width = root.width.takeIf { it > 0 } ?: 1080
+        val height = root.height.takeIf { it > 0 } ?: 1920
+        val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888)
+        root.draw(Canvas(bitmap))
+        val cropped = bitmap.trimmedToDrawnArea()
+
+        val outDir = File("build/screenshots").apply { mkdirs() }
+        val file = File(outDir, name)
+        file.outputStream().use { cropped.compress(Bitmap.CompressFormat.PNG, 100, it) }
+        println("SCREENSHOT_WRITTEN: ${file.absolutePath} (${cropped.width}x${cropped.height})")
+    }
+
+    /** Crop away the untouched (fully transparent) margin, so the PNG is just what was composed. */
+    private fun Bitmap.trimmedToDrawnArea(): Bitmap {
+        val pixels = IntArray(width * height)
+        getPixels(pixels, 0, width, 0, 0, width, height)
+        var left = width
+        var top = height
+        var right = -1
+        var bottom = -1
+        for (y in 0 until height) {
+            for (x in 0 until width) {
+                if (pixels[y * width + x] ushr 24 == 0) continue
+                if (x < left) left = x
+                if (x > right) right = x
+                if (y < top) top = y
+                if (y > bottom) bottom = y
+            }
+        }
+        if (right < left || bottom < top) return this
+        return Bitmap.createBitmap(this, left, top, right - left + 1, bottom - top + 1)
+    }
+}

apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RowGapTest.kt

@@ -0,0 +1,83 @@
+package com.flipcash.app.messenger.internal
+
+import com.flipcash.app.messenger.internal.screens.components.RowGap
+import com.flipcash.app.messenger.internal.screens.components.rowGapBelow
+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.minutes
+import kotlin.time.Instant
+
+/**
+ * How far apart two rows sit.
+ *
+ * The list is `reverseLayout`, so the gap a row owns is the one under it — to the *newer* message,
+ * `peek(index - 1)`. A change of author opens the widest gap, which is the space the next run's
+ * name and picture go in.
+ */
+class RowGapTest {
+
+    private val config = SeparatorConfig.DayOnly
+    private val noon = Instant.fromEpochMilliseconds(1_700_000_000_000)
+
+    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,
+        at: Instant = noon,
+    ) = ChatListItem.ContentBubble(
+        messageId = 1L,
+        contentIndex = 0,
+        content = MessageContent.Text("hi"),
+        isFromSelf = isFromSelf,
+        timestamp = at,
+        sender = sender,
+    )
+
+    @Test
+    fun `two members of a group are a run apart`() {
+        assertEquals(RowGap.Wide, rowGapBelow(bubble(alice), bubble(bob), config))
+    }
+
+    @Test
+    fun `the viewer and a member are a run apart`() {
+        val own = bubble(sender = null, isFromSelf = true)
+        assertEquals(RowGap.Wide, rowGapBelow(bubble(alice), own, config))
+        assertEquals(RowGap.Wide, rowGapBelow(own, bubble(alice), config))
+    }
+
+    @Test
+    fun `one member's consecutive messages stay tight`() {
+        assertEquals(RowGap.Tight, rowGapBelow(bubble(alice), bubble(alice), config))
+    }
+
+    @Test
+    fun `one member's messages open up outside the grouping window`() {
+        val later = bubble(alice, at = noon + 5.minutes)
+        assertEquals(RowGap.Normal, rowGapBelow(bubble(alice), later, config))
+    }
+
+    @Test
+    fun `the viewer's own consecutive messages stay tight`() {
+        val own = bubble(sender = null, isFromSelf = true)
+        assertEquals(RowGap.Tight, rowGapBelow(own, own, config))
+    }
+
+    @Test
+    fun `a separator takes the normal gap on either side`() {
+        val separator = ChatListItem.DateSeparator(noon)
+        assertEquals(RowGap.Normal, rowGapBelow(bubble(alice), separator, config))
+        assertEquals(RowGap.Normal, rowGapBelow(separator, bubble(alice), config))
+    }
+
+    @Test
+    fun `the newest row has nothing under it`() {
+        assertEquals(RowGap.Tight, rowGapBelow(bubble(alice), below = null, config = config))
+    }
+}

apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt

@@ -49,6 +49,21 @@ sealed interface ChatListItem {
          */
         val sender: SenderIdentity? = null,
     ) : ChatListItem {
+        /**
+         * 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.
+         */
+        fun isSameAuthorAs(other: ContentBubble): Boolean = when {
+            isFromSelf != other.isFromSelf -> false
+            isFromSelf -> true
+            else -> sender?.userId == other.sender?.userId
+        }
+
         /** The body a Copy or an Edit acts on, or `null` for a bubble that carries no text. */
         val plainText: String?
             get() = when (val content = content) {