6034 - Fix duplicate LazyGrid keys in ChannelMediaAttachmentsGrid

[amanna13]

🎯 Goal

Fix a crash in ChannelMediaAttachmentsGrid caused by duplicate LazyVerticalGrid keys during pagination and recomposition.

This PR addresses issue #6034.

🛠 Implementation details

ChannelMediaAttachmentsGrid uses LazyVerticalGrid with paginated data and a conditional loading item.
Previously, grid items used item.id as the sole key:

        LazyVerticalGrid(
            modifier = Modifier.matchParentSize(),
            columns = GridCells.Fixed(gridColumnCount),
            verticalArrangement = Arrangement.spacedBy(ChatTheme.dimens.attachmentsContentMediaGridSpacing),
            horizontalArrangement = Arrangement.spacedBy(ChatTheme.dimens.attachmentsContentMediaGridSpacing),
            state = gridState,
        ) {
            itemsIndexed(
                items = content.items,
                key = { _, item -> item.id },
            ) { index, item ->
                itemContent(index, item) {
                    previewItemIndex = index
                }
            }
            if (content.isLoadingMore) {
                item { loadingItem() }
            }
        }

This is unsafe in this context :

1. item.id is not guaranteed to be globally unique

   • Multiple attachments can originate from the same message
   • Paging can merge items from different pages that share the same logical ID
   • During recomposition, two distinct grid items may temporarily resolve to the same key

2. The loading item had no explicit key

   • When no key is provided, Compose generates position-based keys
   • Because the loading item is conditionally inserted/removed, the structure of the grid changes
   • This causes auto-generated keys for neighboring items to shift
   • During fast scrolling and pagination, this can result in duplicate keys being present at the same time

LazyVerticalGrid requires keys to be unique among all currently composed items, including transient items created during paging and recomposition. When this contract is violated, Compose throws an IllegalArgumentException for duplicate keys.

Fix

• Grid item keys are now made explicitly unique by combining the item identity with its position:

key = { index, item -> "${item.id}-$index" }

This preserves stable identity while guaranteeing uniqueness during paging and recomposition.

• The loading item is given a stable, explicit key:

item(key = "loading_item") { ... }

This prevents key shifting when the loading item is added or removed.

These changes ensure that all grid items - including transient loading items, have stable and unique keys across recompositions and pagination.

🎨 UI Changes

NA

🧪 Testing

• Ran the Compose sample app
• Created a direct message channel
• Sent a large number of media attachments
• Opened the Media Attachments grid
• Triggered pagination via fast scrolling and orientation changes
• Verified no crashes occur after the fix

Summary by CodeRabbit

• Bug Fixes
  • Improved stability of attachment handling by refining how attachment items are identified internally, eliminating potential issues with undefined values.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR modifies item ID generation in ChannelAttachmentsViewState by removing the dependency on getDisplayableName() and simplifying ID construction to use only message.identifierHash() and attachment.imagePreviewUrl. This eliminates a potentially undefined fallback and related import.

Changes

Cohort / File(s)	Summary
ID Generation Simplification stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/channel/attachments/ChannelAttachmentsViewState.kt	Changed Item.id initializer from "${message.identifierHash()}-${attachment.getDisplayableName() ?: attachment.imagePreviewUrl}" to "${message.identifierHash()}-${attachment.imagePreviewUrl}", removing the getDisplayableName() fallback and its import.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Single targeted change: Only one property initializer modified in one file
• Clear logic simplification: Removes a conditional fallback that could introduce ambiguity
• Verify impact: Confirm that attachment.imagePreviewUrl is always non-null or appropriately handles cases where it was previously compensated by getDisplayableName()

Possibly related issues

• [BUG] ChannelMediaAttachmentsScreen crashes with duplicate item.id in LazyGrid #6034: The simplified item ID construction directly addresses duplicate key generation in LazyGrid by removing the unreliable getDisplayableName() fallback that could produce identical IDs for different attachments.

Poem

🐰 A tangled ID, once broke and frayed,
Now dances clean in the sun's light displayed,
No fallback shenanigans, just hashes pure,
Duplicates vanish—a remedy sure! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing duplicate LazyGrid keys in ChannelMediaAttachmentsGrid.
Description check	✅ Passed	The description covers Goal, Implementation details, and Testing sections with comprehensive explanations, though UI Changes is marked NA and some checklist items are incomplete.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 586e7c8 and e223ab0.

📒 Files selected for processing (1)

• stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/channel/attachments/ChannelAttachmentsViewState.kt (1 hunks)

[amanna13]

It turns out imagePreviewUrl alone is not guaranteed to be unique per attachment. Multiple attachments (especially in tests or reused media) can reference the same preview URL, which still results in duplicate LazyGrid keys.

@andremion To address this, I guess the attachment item ID must also include the attachment’s position within the message. This preserves a stable, deterministic identity for each attachment instance while avoiding UI-derived or non-deterministic fallbacks.

public val id: String =
    "${message.identifierHash()}-${attachment.imagePreviewUrl}-${message.attachments.indexOf(attachment)}"

This should resolve the remaining duplicate key crashes observed in Compose UI tests.