give image cluster priority over the emoji for group conversations

Convos/Conversations List/View Controller/ConversationsViewController.swift

@@ -171,12 +171,19 @@ final class ConversationsViewController: UIViewController {
                 changed.insert(id)
                 continue
             }
+            // `avatarType` encodes the rendered avatar (the clustered member
+            // profiles and their avatar URLs), so comparing it reconfigures the
+            // cell when a member changes their photo. Without this, a member
+            // avatar update re-emits a fresh conversation but no tracked field
+            // here changes, so every group showing that member keeps a stale
+            // cluster until something else about the row changes.
             if oldConvo.isMuted != newConvo.isMuted ||
                 oldConvo.isUnread != newConvo.isUnread ||
                 oldConvo.isPinned != newConvo.isPinned ||
                 oldConvo.scheduledExplosionDate != newConvo.scheduledExplosionDate ||
                 oldConvo.displayName != newConvo.displayName ||
-                oldConvo.lastMessage != newConvo.lastMessage {
+                oldConvo.lastMessage != newConvo.lastMessage ||
+                oldConvo.avatarType != newConvo.avatarType {
                 changed.insert(id)
             }
         }

ConvosCore/Sources/ConvosCore/Storage/Models/Conversation.swift

@@ -245,14 +245,16 @@ public extension Conversation {
         if otherMembers.count == 1, let member = otherMembers.first {
             return .profile(member.profile, member.agentVerification)
         }
-        if let conversationEmoji, !conversationEmoji.isEmpty {
-            return .emoji(conversationEmoji)
-        }
+        // A group whose members have avatars renders the combined cluster of
+        // their photos. This is checked before the conversation emoji because
+        // that emoji is only ever an auto-seeded fallback (there is no UI to
+        // choose it), so it should show only when there is nothing better - not
+        // suppress the member cluster.
         let otherProfiles = otherMembers.map(\.profile)
-        if otherProfiles.isEmpty || !otherProfiles.hasAnyAvatar {
-            return .emoji(defaultEmoji)
+        if !otherProfiles.isEmpty, otherProfiles.hasAnyAvatar {
+            return .clustered(Array(otherProfiles.sortedForCluster().prefix(7)))
         }
-        return .clustered(Array(otherProfiles.sortedForCluster().prefix(7)))
+        return .emoji(defaultEmoji)
     }
 
     var memberNamesString: String {

ConvosCore/Tests/ConvosCoreTests/ConversationsRepositoryAvatarObservationTests.swift

@@ -0,0 +1,161 @@
+@testable import ConvosCore
+import Combine
+import Foundation
+import GRDB
+import Testing
+
+/// Determines whether the conversation-list `ValueObservation` re-fires when a
+/// member's canonical `profileAvatar` changes.
+///
+/// The list joins each member's avatar through the `profileAvatarLatest` *view*
+/// (`DBConversationMember.avatarSlot`), fetched via an `.including(all:)`
+/// prefetch. Whether GRDB's region tracking reaches the underlying `profileAvatar`
+/// table through that view is the open question behind the "stale cluster until
+/// you open the conversation" bug:
+///
+/// - If `reEmitsOnMemberProfileAvatarWrite` passes, the data layer is already
+///   reactive and the stale-cluster bug is a UI cell-reload problem (fix the
+///   list/diffable-data-source, not the observation).
+/// - If it fails while the `reEmitsOnConversationChange` control passes, the
+///   observation is not tracking `profileAvatar` through the view and needs an
+///   explicit tracked region.
+@Suite("Conversation list observation - profileAvatar reactivity", .serialized)
+struct ConversationsRepositoryAvatarObservationTests {
+    private let salt = Data(repeating: 1, count: 32)
+    private let nonce = Data(repeating: 2, count: 12)
+    private let key = Data(repeating: 3, count: 32)
+
+    private func seedGroup(_ db: Database) throws {
+        try DBMember(inboxId: "me").save(db, onConflict: .ignore)
+        try DBInbox(inboxId: "me", clientId: "client-me", createdAt: Date()).save(db, onConflict: .ignore)
+        try DBMember(inboxId: "alice").save(db, onConflict: .ignore)
+
+        try DBConversation(
+            id: "c1",
+            clientConversationId: "client-c1",
+            inviteTag: "tag-c1",
+            creatorId: "me",
+            kind: .group,
+            consent: .allowed,
+            createdAt: Date(),
+            name: nil,
+            description: nil,
+            imageURLString: nil,
+            publicImageURLString: nil,
+            includeInfoInPublicPreview: false,
+            expiresAt: nil,
+            debugInfo: .empty,
+            isLocked: false,
+            imageSalt: nil,
+            imageNonce: nil,
+            imageEncryptionKey: nil,
+            conversationEmoji: nil,
+            imageLastRenewed: nil,
+            isUnused: false,
+            hasHadVerifiedAgent: false
+        ).insert(db)
+
+        try ConversationLocalState(
+            conversationId: "c1",
+            isPinned: false,
+            isUnread: false,
+            isUnreadUpdatedAt: Date(),
+            isMuted: false,
+            pinnedOrder: nil,
+            hidesInviteCard: false,
+            leftHostedInviteSession: false,
+            wasRemoved: false,
+            hasHadOtherMembers: false,
+            hasSharedInvite: false
+        ).insert(db)
+
+        for (inboxId, role) in [("me", MemberRole.superAdmin), ("alice", MemberRole.member)] {
+            try DBConversationMember(
+                conversationId: "c1",
+                inboxId: inboxId,
+                role: role,
+                consent: .allowed,
+                createdAt: Date(),
+                invitedByInboxId: nil
+            ).insert(db)
+        }
+        try DBProfile(
+            inboxId: "alice", name: "Alice", profileSource: .profileUpdate,
+            updatedAt: Date(timeIntervalSince1970: 1)
+        ).save(db)
+    }
+
+    @Test("re-emits when a member's profileAvatar is written (through the profileAvatarLatest view)")
+    func reEmitsOnMemberProfileAvatarWrite() async throws {
+        let dbManager = MockDatabaseManager.makeTestDatabase()
+        let writer = dbManager.dbWriter
+        try await writer.write { db in try self.seedGroup(db) }
+
+        let counter = EmissionCounter()
+        let repo = ConversationsRepository(dbReader: writer, consent: [.allowed])
+        let cancellable = repo.conversationsPublisher.sink { _ in counter.increment() }
+        defer { cancellable.cancel() }
+
+        try await Task.sleep(nanoseconds: 300_000_000)
+        let afterInitial = counter.count
+        #expect(afterInitial >= 1, "Observation should deliver an initial value")
+
+        // Simulate Alice updating her photo: a new canonical avatar row.
+        try await writer.write { db in
+            try DBProfileAvatar(
+                inboxId: "alice", conversationId: "c1",
+                url: "https://example.com/alice-new.bin",
+                salt: self.salt, nonce: self.nonce, encryptionKey: self.key,
+                profileSource: .profileUpdate, updatedAt: Date()
+            ).save(db)
+        }
+
+        try await Task.sleep(nanoseconds: 800_000_000)
+        #expect(
+            counter.count > afterInitial,
+            "Expected the conversation-list observation to re-emit when a member's profileAvatar changed"
+        )
+    }
+
+    @Test("control: re-emits when the conversation row itself changes")
+    func reEmitsOnConversationChange() async throws {
+        let dbManager = MockDatabaseManager.makeTestDatabase()
+        let writer = dbManager.dbWriter
+        try await writer.write { db in try self.seedGroup(db) }
+
+        let counter = EmissionCounter()
+        let repo = ConversationsRepository(dbReader: writer, consent: [.allowed])
+        let cancellable = repo.conversationsPublisher.sink { _ in counter.increment() }
+        defer { cancellable.cancel() }
+
+        try await Task.sleep(nanoseconds: 300_000_000)
+        let afterInitial = counter.count
+        #expect(afterInitial >= 1, "Observation should deliver an initial value")
+
+        try await writer.write { db in
+            guard let conversation = try DBConversation.fetchOne(db, key: "c1") else { return }
+            try conversation.with(name: "Renamed").save(db)
+        }
+
+        try await Task.sleep(nanoseconds: 800_000_000)
+        #expect(
+            counter.count > afterInitial,
+            "Control failed: the observation did not re-emit even for a direct conversation change"
+        )
+    }
+}
+
+private final class EmissionCounter: @unchecked Sendable {
+    private let lock = NSLock()
+    private var value = 0
+    func increment() {
+        lock.lock()
+        value += 1
+        lock.unlock()
+    }
+    var count: Int {
+        lock.lock()
+        defer { lock.unlock() }
+        return value
+    }
+}

ConvosCore/Tests/ConvosCoreTests/DefaultConversationDisplayTests.swift

@@ -547,6 +547,68 @@ struct DefaultConversationDisplayTests {
         }
     }
 
+    @Test("avatarType clusters a group with member avatars even when a conversation emoji is seeded")
+    func avatarTypeClustersOverSeededConversationEmoji() {
+        // The conversation emoji is only ever an auto-seeded fallback (there is
+        // no UI to choose it), so it must not suppress the member cluster - the
+        // #686 introduced checking the emoji before the cluster, but then this decision
+        // was reversed. 
+        let members = [
+            ConversationMember.mock(isCurrentUser: true, name: "You"),
+            ConversationMember(
+                profile: Profile.mock(inboxId: "other1", name: "Alice"),
+                role: .member,
+                isCurrentUser: false
+            ),
+            ConversationMember(
+                profile: Profile.mock(inboxId: "other2", name: "Bob"),
+                role: .member,
+                isCurrentUser: false
+            )
+        ]
+        let conversation = Conversation(
+            id: "test",
+            clientConversationId: "client-test",
+            creator: .mock(isCurrentUser: true),
+            createdAt: Date(),
+            consent: .allowed,
+            kind: .group,
+            name: nil,
+            description: nil,
+            members: members,
+            otherMember: nil,
+            messages: [],
+            isPinned: false,
+            isUnread: false,
+            isMuted: false,
+            pinnedOrder: nil,
+            hidesInviteCard: false,
+            leftHostedInviteSession: false,
+            wasRemoved: false,
+            lastMessage: nil,
+            imageURL: nil,
+            imageSalt: nil,
+            imageNonce: nil,
+            imageEncryptionKey: nil,
+            conversationEmoji: "🦊",
+            includeInfoInPublicPreview: false,
+            isDraft: false,
+            invite: nil,
+            expiresAt: nil,
+            debugInfo: .empty,
+            isLocked: false,
+            agentJoinStatus: nil,
+            hasHadVerifiedAgent: false,
+            wasCreatedFromAgentBuilder: false
+        )
+
+        if case .clustered(let profiles) = conversation.avatarType {
+            #expect(profiles.count == 2)
+        } else {
+            #expect(Bool(false), "Expected clustered avatar type - a seeded conversation emoji must not suppress the member cluster")
+        }
+    }
+
     // MARK: - Member Name Limiting Tests
 
     @Test("Four named profiles shows three and 1 other")