fix(ui): prevent LazyColumn duplicate-key crash in contact lists (COLUMBA-99)

[sentry]

This PR addresses IllegalArgumentException: Key "<hash>" was already used occurring in LazyColumns/LazyRows that display lists of contacts or announces.

Root Cause:
The ContactEntity in the database uses a composite primary key [destinationHash, identityHash]. This means it's legitimate for two different ContactEntity records (belonging to different local identities) to share the same destinationHash. However, several UI components were using destinationHash alone as the key for LazyColumn items. When a list contained two items with the same destinationHash, Compose would crash due to the non-unique key.

Solution:
To resolve this, distinctBy { it.destinationHash } has been applied to the data lists (e.g., pinnedContacts, allContacts, availableRelays, savedPeers) immediately before they are passed to the LazyColumn's items function. This ensures that only unique destinationHash values are used as keys within a single list, preventing the IllegalArgumentException.

Affected Files:

• app/src/main/java/network/columba/app/ui/screens/ContactsScreen.kt
• app/src/main/java/network/columba/app/ui/screens/settings/cards/MessageDeliveryRetrievalCard.kt
• app/src/main/java/network/columba/app/ui/screens/SavedPeersScreen.kt

ShareLocationBottomSheet.kt and LocationSharingCard.kt were also identified as potentially problematic but already contained similar distinctBy deduplication logic, so no changes were required there.

This fix is a pragmatic solution given the current data models, ensuring UI stability when handling contacts that may share destination hashes across different local identities.

Fixes COLUMBA-99

[sentry]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

This PR fixes an IllegalArgumentException crash in three LazyColumn screens by applying distinctBy { it.destinationHash } to deduplicate lists before they are passed to items(…, key = …). The crash was caused by ContactEntity's composite PK [destinationHash, identityHash] allowing multiple rows with the same destinationHash to appear in a single list.

• ContactsScreen: Deduplication uses .lowercase() consistently in both the filter and the key lambda, but silently drops contacts that belong to different local identities sharing a destination hash — actions (delete, pin, chat) will only affect whichever record appears first.
• SavedPeersScreen / MessageDeliveryRetrievalCard: Deduplication omits .lowercase() normalization present in ContactsScreen, leaving a gap where mixed-case hashes bypass distinctBy and can still produce duplicate keys and crash the LazyColumn.

Confidence Score: 3/5

The crash fix is effective for the common case, but two of three files omit the lowercase normalization that the third applies, leaving those screens able to re-crash if hashes arrive with mixed casing.

Two files (SavedPeersScreen and MessageDeliveryRetrievalCard) skip the .lowercase() normalization that ContactsScreen uses, meaning the same crash condition can reoccur for those screens with mixed-case hashes. Additionally, distinctBy on destinationHash alone discards entire ContactEntity rows that are legitimate, distinct DB records — a user with the same destination under two identities sees only one entry and any action they take operates only on whichever record was first in the result set.

SavedPeersScreen.kt and MessageDeliveryRetrievalCard.kt both need .lowercase() added to their distinctBy selectors. ContactsScreen.kt would benefit from a longer-term fix using a composite key instead of filtering rows out entirely.

Important Files Changed

Filename	Overview
app/src/main/java/network/columba/app/ui/screens/ContactsScreen.kt	Adds distinctBy { it.destinationHash.lowercase() } before both pinned and all-contacts LazyColumn sections to prevent the duplicate-key crash; silently drops contacts sharing a destinationHash across identities, and the section-header guard for ALL CONTACTS is updated to use the deduplicated list.
app/src/main/java/network/columba/app/ui/screens/SavedPeersScreen.kt	Introduces distinctBy { it.destinationHash } (without .lowercase()) before the items call — inconsistent with ContactsScreen and leaves the crash possible if hashes arrive with mixed casing.
app/src/main/java/network/columba/app/ui/screens/settings/cards/MessageDeliveryRetrievalCard.kt	Moves the distinctBy call inside the LazyListScope block and omits .lowercase() — same case-folding gap as SavedPeersScreen, so a mixed-case hash could bypass deduplication and re-trigger the crash.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Raw contact/peer list from ViewModel] --> B{distinctBy destinationHash}
    B -->|First occurrence kept| C[Deduplicated list]
    B -->|Later duplicates| D[Silently dropped from UI]
    C --> E[LazyColumn items with key=destinationHash]
    E --> F[No duplicate-key crash]
    D --> G[Contact hidden from user\nActions on visible item\nmay miss other identity records]

    style D fill:#ffcccc,stroke:#cc0000
    style G fill:#ffcccc,stroke:#cc0000
    style F fill:#ccffcc,stroke:#009900

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
app/src/main/java/network/columba/app/ui/screens/ContactsScreen.kt:522-523
**`distinctBy` silently drops legitimate contact records**

When a user has the same destination registered under multiple local identities, `distinctBy { it.destinationHash }` keeps only the first occurrence and quietly discards the rest. This means UI-triggered actions (delete, pin, open chat) will operate on whichever `ContactEntity` happens to be first in iteration order, silently ignoring the other identity's record. The root cause is that the composite PK `[destinationHash, identityHash]` means these are genuinely distinct rows. The crash-safe fix is to expose `identityHash` through the UI model and build a composite key — `"pinned_${contact.destinationHash.lowercase()}_${contact.identityHash.lowercase()}"` — rather than filtering out rows that the user has explicitly created.

### Issue 2 of 3
app/src/main/java/network/columba/app/ui/screens/SavedPeersScreen.kt:88
The `distinctBy` calls in `SavedPeersScreen` and `MessageDeliveryRetrievalCard` omit the `.lowercase()` normalization that `ContactsScreen` applies. If a `destinationHash` ever arrives with inconsistent casing (e.g., `"ABC123"` vs `"abc123"`), `distinctBy` would treat them as distinct — meaning no deduplication and a possible surviving duplicate key that can still crash the `LazyColumn`. Align with the `ContactsScreen` approach by normalizing to lowercase before `distinctBy`.

```suggestion
                val uniqueSavedPeers = savedPeers.distinctBy { it.destinationHash.lowercase() }
```

### Issue 3 of 3
app/src/main/java/network/columba/app/ui/screens/settings/cards/MessageDeliveryRetrievalCard.kt:958
Same missing lowercase normalization as in `SavedPeersScreen`. `distinctBy { it.destinationHash }` without case-folding leaves a crack open: if the same relay is announced with mixed-case hashes the deduplication silently fails and the crash can recur. Apply `.lowercase()` to match the pattern already established in `ContactsScreen`.

```suggestion
                    val uniqueRelays = availableRelays.distinctBy { it.destinationHash.lowercase() }
```

_{Reviews (1): Last reviewed commit: "fix(ui): prevent LazyColumn duplicate-ke..." | Re-trigger Greptile}

[greptile-apps]

distinctBy silently drops legitimate contact records

When a user has the same destination registered under multiple local identities, distinctBy { it.destinationHash } keeps only the first occurrence and quietly discards the rest. This means UI-triggered actions (delete, pin, open chat) will operate on whichever ContactEntity happens to be first in iteration order, silently ignoring the other identity's record. The root cause is that the composite PK [destinationHash, identityHash] means these are genuinely distinct rows. The crash-safe fix is to expose identityHash through the UI model and build a composite key — "pinned_${contact.destinationHash.lowercase()}_${contact.identityHash.lowercase()}" — rather than filtering out rows that the user has explicitly created.

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/src/main/java/network/columba/app/ui/screens/ContactsScreen.kt
Line: 522-523

Comment:
**`distinctBy` silently drops legitimate contact records**

When a user has the same destination registered under multiple local identities, `distinctBy { it.destinationHash }` keeps only the first occurrence and quietly discards the rest. This means UI-triggered actions (delete, pin, open chat) will operate on whichever `ContactEntity` happens to be first in iteration order, silently ignoring the other identity's record. The root cause is that the composite PK `[destinationHash, identityHash]` means these are genuinely distinct rows. The crash-safe fix is to expose `identityHash` through the UI model and build a composite key — `"pinned_${contact.destinationHash.lowercase()}_${contact.identityHash.lowercase()}"` — rather than filtering out rows that the user has explicitly created.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

The distinctBy calls in SavedPeersScreen and MessageDeliveryRetrievalCard omit the .lowercase() normalization that ContactsScreen applies. If a destinationHash ever arrives with inconsistent casing (e.g., "ABC123" vs "abc123"), distinctBy would treat them as distinct — meaning no deduplication and a possible surviving duplicate key that can still crash the LazyColumn. Align with the ContactsScreen approach by normalizing to lowercase before distinctBy.

Suggested change

	val uniqueSavedPeers = savedPeers.distinctBy { it.destinationHash }
	val uniqueSavedPeers = savedPeers.distinctBy { it.destinationHash.lowercase() }

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/src/main/java/network/columba/app/ui/screens/SavedPeersScreen.kt
Line: 88

Comment:
The `distinctBy` calls in `SavedPeersScreen` and `MessageDeliveryRetrievalCard` omit the `.lowercase()` normalization that `ContactsScreen` applies. If a `destinationHash` ever arrives with inconsistent casing (e.g., `"ABC123"` vs `"abc123"`), `distinctBy` would treat them as distinct — meaning no deduplication and a possible surviving duplicate key that can still crash the `LazyColumn`. Align with the `ContactsScreen` approach by normalizing to lowercase before `distinctBy`.

```suggestion
                val uniqueSavedPeers = savedPeers.distinctBy { it.destinationHash.lowercase() }
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Same missing lowercase normalization as in SavedPeersScreen. distinctBy { it.destinationHash } without case-folding leaves a crack open: if the same relay is announced with mixed-case hashes the deduplication silently fails and the crash can recur. Apply .lowercase() to match the pattern already established in ContactsScreen.

Suggested change

	val uniqueRelays = availableRelays.distinctBy { it.destinationHash }
	val uniqueRelays = availableRelays.distinctBy { it.destinationHash.lowercase() }

Prompt To Fix With AI

This is a comment left during a code review.
Path: app/src/main/java/network/columba/app/ui/screens/settings/cards/MessageDeliveryRetrievalCard.kt
Line: 958

Comment:
Same missing lowercase normalization as in `SavedPeersScreen`. `distinctBy { it.destinationHash }` without case-folding leaves a crack open: if the same relay is announced with mixed-case hashes the deduplication silently fails and the crash can recur. Apply `.lowercase()` to match the pattern already established in `ContactsScreen`.

```suggestion
                    val uniqueRelays = availableRelays.distinctBy { it.destinationHash.lowercase() }
```

How can I resolve this? If you propose a fix, please make it concise.