app/src/main/java/com/lxmf/messenger/viewmodel/MapViewModel.kt, line 473
pendingFocusContact is never consumed — camera animation is dead code

focusOnContact() and consumePendingFocus() are added to MapViewModel, and MapScreen is now passed the shared mapViewModel instance, but MapScreen.kt is not included in this PR's changeset. A search through the file confirms it contains no reference to pendingFocusContact, consumePendingFocus, or any LaunchedEffect that reacts to the new state flow.

As a result, clicking "Locate on Map" from any of the three entry points will navigate the user to the Map tab correctly, but the camera will never animate to the contact's marker. The pendingFocusContact value is set and then sits unconsumed indefinitely.

MapScreen.kt needs a LaunchedEffect similar to:

val pendingFocus by viewModel.pendingFocusContact.collectAsState()
LaunchedEffect(pendingFocus) {
    pendingFocus?.let { hash ->
        val marker = state.contactMarkers.find {
            it.destinationHash.equals(hash, ignoreCase = true)
        }
        marker?.let {
            cameraPositionState.animate(
                CameraUpdateFactory.newLatLngZoom(LatLng(it.latitude, it.longitude), 15f)
            )
        }
        viewModel.consumePendingFocus()
    }
}

Without this, the core user-visible behaviour of the feature (focusing the map on the contact) is missing.

app/src/main/java/com/lxmf/messenger/ui/screens/ContactsScreen.kt, line 226
Index-based key defeats the purpose of stable Lazy list keys

The key for the all contacts list was changed from "all_${contact.destinationHash.lowercase()}" to "all_${contact.destinationHash}_$index". Including the positional index in the key means that whenever any item is inserted, removed, or reordered, every subsequent item gets a brand-new key. Compose then considers all of those items as entirely new composables — discarding their state and recreating them — which negates the performance benefit of using key at all and can cause visible recomposition flicker.

If the motivation was to avoid a duplicate-key crash when two contacts share the same destination hash (with different casing), a simpler fix that keeps stable keys would be:

                                itemsIndexed(
                                    contactsState.groupedContacts.all,
                                    key = { _, contact -> "all_${contact.destinationHash.lowercase()}" },
                                ) { _, contact ->

The .lowercase() already made the key case-insensitive. If true duplicates can appear in groupedContacts.all, the real fix belongs in the data layer rather than using a position-based key.

app/src/main/java/com/lxmf/messenger/ui/screens/ChatsScreen.kt, line 121
Stale contactLocation persists after menu dismissal

contactLocation is only populated when showMenu becomes true, but it is never cleared when showMenu becomes false. If a contact broadcasts a location, the user opens the context menu (sees "Locate on Map"), closes it without tapping, the contact then stops sharing their location (e.g. the entry expires), and the user reopens the menu — the stale Pair is still in state, so "Locate on Map" is shown again until the new LaunchedEffect coroutine completes.

Resetting the value on close prevents this race:

                        var contactLocation by remember { mutableStateOf<Pair<Double, Double>?>(null) }

                        // Fetch contact location when menu opens; clear on close
                        LaunchedEffect(showMenu) {
                            if (showMenu) {
                                contactLocation = viewModel.getContactLocation(conversation.peerHash)
                            } else {
                                contactLocation = null
                            }
                        }

The same pattern applies to hasLocation in ContactListItemWithMenu in ContactsScreen.kt.

app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt, line 355
consumePendingFocus() on line 354 is called unconditionally — even when marker is null. This creates a silent failure: if state.contactMarkers is empty or hasn't yet been populated when this LaunchedEffect fires (e.g., because the database query backing the contactMarkers StateFlow hasn't emitted yet), the pending focus is consumed without ever animating the camera.

The race condition occurs because:

• focusOnContact(peerHash) is called before navigating to the Map screen
• Once the Map composable recomposes, pendingFocus emits and this effect runs
• If the markers Flow hasn't emitted the latest data yet, find() returns null
• consumePendingFocus() fires anyway, and the user sees the map with no camera movement
• There is no retry since pendingFocus is now null and won't change again

Fix: Only consume the focus after a successful animation:

marker?.let {
    val cameraPosition = CameraPosition.Builder()
        .target(LatLng(it.latitude, it.longitude))
        .zoom(15.0)
        .build()
    map.animateCamera(CameraUpdateFactory.newCameraPosition(cameraPosition))
    viewModel.consumePendingFocus()  // ← move inside let block
}

Alternatively, add state.contactMarkers as a key to the LaunchedEffect so it re-fires when markers are loaded.

app/src/main/java/com/lxmf/messenger/viewmodel/MessagingViewModel.kt, line 356
observeLatestLocationForSender() returns the most recent DB row for the sender regardless of its expiresAt value. As a result, hasContactLocation can be true — showing the "Locate on Map" button — for a contact whose location has expired but hasn't been cleaned up yet (the cleanup job keeps expired rows for a 1-hour grace period).

Meanwhile, the map's contactMarkers StateFlow is derived from getLatestLocationsPerSenderUnfiltered() passed through calculateMarkerState(), which can exclude a marker if it's considered hidden (expired beyond the grace period). If the expired location falls into a hidden state, the user taps "Locate on Map", is navigated to the Map screen, and finds no marker to focus on (causing the race condition from the MapScreen comment).

Fix: Add an expiry check to the mapping:

receivedLocationDao.observeLatestLocationForSender(hash)
    .map { it != null && (it.expiresAt == null || it.expiresAt > System.currentTimeMillis()) }

The same expiry mismatch exists in ChatsViewModel.getContactLocation() and ContactsViewModel.getContactLocation(), which also use the unfiltered getLatestLocationForSender() query.

app/src/main/java/com/lxmf/messenger/viewmodel/ChatsViewModel.kt, line 434
Missing case normalization on DAO lookup

getLatestLocationForSender uses a raw SQL WHERE senderHash = :senderHash (case-sensitive in SQLite). LocationSharingManager now explicitly stores hashes as lowercase (via .lowercase()). If peerHash arriving here from the UI is not already lowercase, this query will return null even when a record exists, silently hiding the "Locate on Map" option.

The same issue exists in ContactsViewModel.getContactLocation and MessagingViewModel.hasContactLocation.

        val location = receivedLocationDao.getLatestLocationForSender(peerHash.lowercase())

app/src/main/java/com/lxmf/messenger/viewmodel/MessagingViewModel.kt, line 563
Missing case normalization on reactive DAO query

observeLatestLocationForSender(hash) is also a case-sensitive SQL lookup. Since LocationSharingManager now stores senderHash as lowercase, passing a mixed-case hash here will produce an empty flow, keeping hasContactLocation permanently false and hiding the map button in the top bar.

The same root cause affects ChatsViewModel.getContactLocation and ContactsViewModel.getContactLocation (both pass the hash unsanitised to getLatestLocationForSender).

                        receivedLocationDao.observeLatestLocationForSender(hash.lowercase())

app/src/main/java/com/lxmf/messenger/ui/screens/MapScreen.kt, line 347
pendingFocusContact is never consumed when the marker is absent

If the contact's marker has been removed from state.contactMarkers (e.g., their location expired and was cleaned up between the tap and the map rendering), marker will be null on every LaunchedEffect re-run, so consumePendingFocus() is never called. pendingFocusContact stays non-null indefinitely, meaning the next time any contact marker loads or the map re-renders it will attempt a focus lookup again for a hash that may never match.

Consider adding a timeout or consuming the pending focus after a maximum number of retries (or when the contact is confirmed absent from markers):

marker?.let {
    val cameraPosition = CameraPosition.Builder()
        .target(LatLng(it.latitude, it.longitude))
        .zoom(15.0)
        .build()
    map.animateCamera(CameraUpdateFactory.newCameraPosition(cameraPosition))
    viewModel.consumePendingFocus()
} ?: run {
    // If markers have loaded and the hash still isn't found, stop waiting
    if (state.contactMarkers.isNotEmpty() && !state.isLoading) {
        viewModel.consumePendingFocus()
    }
}

app/src/main/java/com/lxmf/messenger/ui/screens/ChatsScreen.kt, line 177
Hardcoded UI strings — use string resources

"Locate on Map" appears hardcoded in ChatsScreen.kt and again in ContactsScreen.kt, and "Locate on map" is hardcoded as a content description in MessagingScreen.kt. These should be extracted to strings.xml for localizability and consistency with the rest of the app.

The same pattern exists in:

• app/src/main/java/com/lxmf/messenger/ui/screens/ContactsScreen.kt (line ~1313)
• app/src/main/java/com/lxmf/messenger/ui/screens/MessagingScreen.kt content description

_{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!}

app/src/main/java/com/lxmf/messenger/viewmodel/ChatsViewModel.kt, line 435
Duplicated getContactLocation logic

The method body in ChatsViewModel.getContactLocation is identical to ContactsViewModel.getContactLocation. Both ViewModels now inject ReceivedLocationDao solely for this purpose. Consider extracting this logic into a shared utility function or a thin wrapper in a repository layer (LocationRepository or ReceivedLocationRepository), keeping both ViewModels lean and preventing future drift between the two implementations.

_{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!}

data/src/main/java/com/lxmf/messenger/data/repository/ReceivedLocationRepository.kt, line 40
observeHasLocation does not re-emit when a location expires

observeHasLocation maps the DAO's Flow, but Room only emits a new item when the underlying database row changes — not when wall-clock time crosses expiresAt. This means:

1. Contact shares location → hasContactLocation emits true → "Locate on Map" button appears
2. The sharing period ends / expiresAt is reached, but no DB row is modified
3. The Flow never re-emits false, so the button stays visible indefinitely
4. The user taps the button → focusOnContact() is set → the map finds no valid marker → the pending focus is silently consumed with no user feedback

The same race can happen in the menu-open path via getContactLocation, but that is a one-shot call so the window is tiny. The reactive path in MessagingScreen is the exposed surface.

Consider adding a periodic ticker (e.g. tickerFlow / combine with a timer) to force a re-evaluation when expiresAt is near, or accept the staleness and ensure the "Locate on Map" tap path shows a user-visible error when no marker is found rather than silently consuming the focus.

app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelImageLoadingTest.kt, line 132
Missing mock stub for observeHasLocation may cause test failures

receivedLocationRepository is created as a strict (non-relaxed) mock. MessagingViewModel.hasContactLocation calls receivedLocationRepository.observeHasLocation(hash) for any non-null _currentConversation value. Because this flow uses SharingStarted.WhileSubscribed(5000), the call is deferred until a subscriber attaches — but any test that provides a destinationHash to the ViewModel and subsequently reads or indirectly subscribes to hasContactLocation will throw MockKException: no answer found for observeHasLocation.

The same gap exists in MessagingViewModelTest.kt at the same setup location.

Add a default stub in the setup to make the mock safe for all tests:

every { receivedLocationRepository.observeHasLocation(any()) } returns flowOf(false)

app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelTest.kt, line 119
Missing mock stub for observeHasLocation may cause test failures

receivedLocationRepository is created as a strict (non-relaxed) mock. MessagingViewModel.hasContactLocation calls receivedLocationRepository.observeHasLocation(hash) for any non-null _currentConversation value. Because this flow uses SharingStarted.WhileSubscribed(5000), the call is deferred until a subscriber attaches — but any test that provides a destinationHash to the ViewModel and subsequently reads or indirectly subscribes to hasContactLocation will throw MockKException: no answer found for observeHasLocation.

Add a default stub in the setup to make the mock safe for all tests:

every { receivedLocationRepository.observeHasLocation(any()) } returns flowOf(false)

app/src/main/java/com/lxmf/messenger/ui/screens/ContactsScreen.kt, line 595
Unnecessary change from items to itemsIndexed

The index introduced by switching to itemsIndexed is discarded ({ _, contact ->). The original items call with its key lambda was already correct and more readable. This change has no functional effect but adds noise and imports itemsIndexed unnecessarily.

                                items(
                                    contactsState.groupedContacts.all,
                                    key = { contact -> "all_${contact.destinationHash.lowercase()}" },
                                ) { contact ->

app/src/main/java/com/lxmf/messenger/viewmodel/MapViewModel.kt, line 635
The KDoc block at lines 609–613 ("Delete a stale marker…") was meant for deleteMarker(), but its placement before focusOnContact() leaves it orphaned. The deleteMarker() function at line 630 now lacks documentation.

        /**
         * Request the map to focus on an existing contact marker.
         * Used by "Locate on Map" from the Chats context menu.
         * The MapScreen will look up the marker and animate the camera to it.
         */
        fun focusOnContact(destinationHash: String) {
            _pendingFocusContact.value = destinationHash
        }

        /**
         * Consume the pending focus after the MapScreen has animated to it.
         */
        fun consumePendingFocus() {
            _pendingFocusContact.value = null
        }

        /**
         * Delete a stale marker by removing all stored locations for the given sender.
         *
         * @param destinationHash The destination hash of the contact whose marker to remove
         */
        fun deleteMarker(destinationHash: String) {

data/src/main/java/com/lxmf/messenger/data/repository/ReceivedLocationRepository.kt, line 40
observeHasLocation is backed by a Room reactive query that re-emits only when the underlying table row changes—not when wall-clock time passes. The expiry check (expires > System.currentTimeMillis()) runs at emission time, so if the Flow last emitted true just before expiry and the row remains unchanged, hasContactLocation in MessagingViewModel can stay true even after the location has expired.

This means the "Locate on Map" icon may remain visible in the top bar until the next database write (e.g., when LocationSharingManager.cleanupExpiredLocations() runs every ~5 minutes). If a user taps it after the grace period ends and the marker is removed from the map, the camera animation will fail silently, leaving no visual feedback.

Consider combining the expiry check with a time-based re-evaluation mechanism (e.g., a ticker Flow or integration with the existing _refreshTrigger) to keep this state current without relying solely on database changes.

data/src/main/java/com/lxmf/messenger/data/repository/ReceivedLocationRepository.kt, line 30
The "Locate on Map" availability checks in getContactLocation() and observeHasLocation() apply strict expiry filtering (expires > currentTime), while MapViewModel.calculateMarkerState() shows markers for up to 1 hour past expiry (grace period). This creates a behavioral mismatch: during the grace period, a marker is visible on the map but cannot be navigated to via "Locate on Map" from any screen.

Consider aligning the logic by either:

1. Allowing "Locate on Map" entries during the grace period (consistent with map display), or
2. Hiding grace-period markers from the map display (strict expiry everywhere)

This would improve UX consistency and clarify intent to users.