From 0394ec2a75dc9abcf2ba9f18d1fc5c0711ef6c45 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Thu, 4 Jun 2026 18:32:32 -0400 Subject: [PATCH] fix(dm): drive window-limit paging off viewport visibility, not row composition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-relay window-limit sentinel placed its advance() effect inside the gap row that currently hosts the marker, so its identity rode that row. Any feed reorder (a live DM bumping a room, or a slow relay dribbling a history page) moved the gap to a different row, tore the keyed LaunchedEffect down and recreated it, and re-fired advance() on a static screen — re-arming stalled/ auth relays into a 15s silence-watchdog storm and risking an unprompted page-back for delivering relays. Hoist the driver above the list: RelayWindowLimitSentinels now holds one stable effect per non-done limit (keyed by lim.key) that watches the LazyListState and fires advance() only when the marker's gap is among the currently visible rows AND it just scrolled into view OR its reached cursor moved (a page landed). A reorder that keeps the marker on the same side of the fold changes neither, so it no longer re-pages. RelayWindowLimitMarkers is now pure UI. Wired in the rooms list (inline) and the conversation (via a new sentinels slot on ChatFeedView). Verified on device: static rooms list drops from a perpetual ~15s re-fire/ silence storm to 2 silence events and only legit cursor-moved advances, while demand-driven paging (page back while the marker is visible) is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../loggedIn/chats/feed/ChatFeedView.kt | 12 ++ .../chats/feed/layouts/RelayReachMarker.kt | 105 +++++++++++++----- .../loggedIn/chats/privateDM/ChatroomView.kt | 16 ++- .../chats/rooms/feed/ChatroomListFeedView.kt | 5 + 4 files changed, 106 insertions(+), 32 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatFeedView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatFeedView.kt index d4a1cc2737..dd6384eb51 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatFeedView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/ChatFeedView.kt @@ -68,6 +68,10 @@ fun RefreshingChatroomFeedView( // createdAt bounds, so a caller (private DMs) can draw per-relay paging markers at the depth each // relay has reached. No-op for callers without per-relay progress. markersInGap: (@Composable (newerCreatedAt: Long?, olderCreatedAt: Long?) -> Unit)? = null, + // Optional hoisted load driver: handed the loaded message list and its scroll state once (above the + // LazyColumn), so a caller (private DMs) can drive demand-driven paging off viewport visibility + // rather than per-row composition. No-op for callers that don't paginate. + sentinels: (@Composable (items: List, listState: LazyListState) -> Unit)? = null, ) { SaveableFeedState(feedContentState, scrollStateKey) { listState -> listStateObserver(listState) @@ -82,6 +86,7 @@ fun RefreshingChatroomFeedView( avoidDraft, olderBoundary, markersInGap, + sentinels, ) } } @@ -98,6 +103,7 @@ fun RenderChatFeedView( avoidDraft: DraftTagState? = null, olderBoundary: (@Composable () -> Unit)? = null, markersInGap: (@Composable (newerCreatedAt: Long?, olderCreatedAt: Long?) -> Unit)? = null, + sentinels: (@Composable (items: List, listState: LazyListState) -> Unit)? = null, ) { val feedState by feed.feedContent.collectAsStateWithLifecycle() @@ -127,6 +133,7 @@ fun RenderChatFeedView( avoidDraft, olderBoundary, markersInGap, + sentinels, ) } } @@ -145,9 +152,14 @@ fun ChatFeedLoaded( avoidDraft: DraftTagState? = null, olderBoundary: (@Composable () -> Unit)? = null, markersInGap: (@Composable (newerCreatedAt: Long?, olderCreatedAt: Long?) -> Unit)? = null, + sentinels: (@Composable (items: List, listState: LazyListState) -> Unit)? = null, ) { val items by loaded.feed.collectAsStateWithLifecycle() + // Hoisted load driver (above the LazyColumn): pages each relay off viewport visibility, so feed + // reorders no longer re-fire paging. The per-gap markers below are pure UI. + sentinels?.invoke(items.list, listState) + LaunchedEffect(items.list.firstOrNull()) { if (listState.firstVisibleItemIndex <= 1) { listState.animateScrollToItem(0) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/layouts/RelayReachMarker.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/layouts/RelayReachMarker.kt index cd7da3264b..44a201dcfa 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/layouts/RelayReachMarker.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/feed/layouts/RelayReachMarker.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text @@ -29,6 +30,8 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.key import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -41,6 +44,8 @@ import com.vitorpamplona.amethyst.R import com.vitorpamplona.amethyst.ui.theme.DividerThickness import com.vitorpamplona.amethyst.ui.theme.HalfPadding import com.vitorpamplona.quartz.utils.Log +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.distinctUntilChanged /** How far one relay has paged into the conversation, for an in-stream progress marker. */ enum class RelayReachState { @@ -61,10 +66,10 @@ data class RelayReach( ) /** - * One relay's window-limit, used to both place a marker and act as the load sentinel for that relay. - * The marker sits at [reachedUntil] (the oldest point the relay has paged to). [advance] pulls that - * relay's next, older page; the renderer fires it while the marker is on screen (see - * [RelayWindowLimitMarkers]). + * One relay's window-limit: places a marker and carries the [advance] that pulls that relay's next, + * older page. The marker sits at [reachedUntil] (the oldest point the relay has paged to); + * [RelayWindowLimitMarkers] draws it and [RelayWindowLimitSentinels] fires [advance] while it is on + * screen. * * @param key stable identity (protocol tag + relay url) so the sentinel survives list reorders. */ @@ -77,17 +82,74 @@ data class RelayWindowLimit( ) /** - * Renders the window-limit markers for the relays whose limit falls in the gap between a newer message - * (at [newerCreatedAt]) and its next-older neighbour (at [olderCreatedAt], null at the oldest end), and - * — this is the load driver — makes each one a **sentinel**: while this gap is composed (i.e. on/near - * screen), it pulls that relay's next page and keeps pulling as each page lands, so a relay pages on - * while its marker stays on screen and stops when a page pushes it off or the user scrolls away. + * Drives demand-driven paging for every limit, **hoisted above the list** so its identity does not ride + * on which row currently hosts the marker. Each non-done limit gets one stable effect (keyed by + * [RelayWindowLimit.key]) that watches the [listState] and pulls that relay's next page when its marker + * is on screen. * - * The sentinel keys on the reached cursor ALONE — never on the relay's reach state. Keying on state - * would re-fire on every `REACHING ⇄ STALLED` flip, so a flaky/auth relay's connection churn would - * re-page the window on a completely static screen. A stalled relay therefore parks until the cursor - * moves or the marker is scrolled back into view (a single retry, not a loop). A done relay shows ✓ and - * drives nothing. + * Why hoisted: the marker for a limit lives in exactly one gap (between the two rows straddling its + * reached cursor). Placing the sentinel *inside* that row made its effect's identity ride the hosting + * row — so any feed reorder (a live DM, or a slow relay dribbling a history page) moved the gap to a + * different row, tore the effect down and recreated it, and re-fired `advance()` on a static screen. + * That re-armed stalled/auth relays into a silence-watchdog storm and could walk a delivering relay + * back a window with no scroll. Hoisting the effect and driving it off **viewport visibility** instead + * of composition presence removes that coupling. + * + * Fires `advance()` when (and only when) the marker's gap is among the currently visible rows AND either + * it just scrolled into view OR its reached cursor moved (a page landed — keep paging while visible). + * A reorder that keeps the marker on the same side of the fold changes neither, so it no longer re-fires. + * A done relay drives nothing. + * + * @param createdAtAt createdAt of the list item at an index (null past the ends / for non-message rows), + * so the visible-gap test mirrors [RelayWindowLimitMarkers]'s placement against only the on-screen rows. + */ +@Composable +fun RelayWindowLimitSentinels( + limits: List, + listState: LazyListState, + createdAtAt: (index: Int) -> Long?, +) { + limits.forEach { lim -> + if (lim.state == RelayReachState.DONE) return@forEach + key(lim.key) { + val reached = rememberUpdatedState(lim.reachedUntil) + val advance = rememberUpdatedState(lim.advance) + val getAt = rememberUpdatedState(createdAtAt) + LaunchedEffect(Unit) { + snapshotFlow { + val r = reached.value + val at = getAt.value + // Visible if any on-screen row is the "newer" side of the gap holding this cursor — + // the same predicate RelayWindowLimitMarkers uses to place the marker, but over the + // visible rows only. + val onScreen = + listState.layoutInfo.visibleItemsInfo.any { info -> + val newer = at(info.index) ?: return@any false + val older = at(info.index + 1) + newer > r && (older == null || older <= r) + } + // Pair so distinctUntilChanged also lets a landed page (r moved) re-fire while visible, + // not just the off→on-screen transition. + onScreen to r + }.distinctUntilChanged() + .collect { (onScreen, r) -> + if (onScreen) { + // One line per sentinel fire — a re-fire LOOP would show the same key firing + // over and over (and whether its reached cursor is drifting). + Log.d("DMPagination") { "marker fire ${lim.key} reachedUntil=$r" } + advance.value() + } + } + } + } + } +} + +/** + * Renders the window-limit markers for the relays whose limit falls in the gap between a newer message + * (at [newerCreatedAt]) and its next-older neighbour (at [olderCreatedAt], null at the oldest end). Pure + * UI: the load driving lives in [RelayWindowLimitSentinels], so this can be (re)placed freely per row on + * every feed reorder without triggering any paging. */ @Composable fun RelayWindowLimitMarkers( @@ -105,21 +167,6 @@ fun RelayWindowLimitMarkers( } if (here.isEmpty()) return - here.forEach { lim -> - if (lim.state != RelayReachState.DONE) { - // Keyed identity so the effect isn't torn down on reorder; keyed on the reached cursor ONLY so - // it re-fires per landed page (continue while visible) but NOT on stall/unstall churn. - key(lim.key) { - LaunchedEffect(lim.reachedUntil) { - // One line per sentinel fire — a re-fire LOOP shows the same key firing over and over - // (and whether its reached cursor is drifting, which would point at a non-pinned floor). - Log.d("DMPagination") { "marker fire ${lim.key} reachedUntil=${lim.reachedUntil}" } - lim.advance() - } - } - } - } - RelayReachMarker(here.map { RelayReach(it.name, it.state) }) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt index cf512326ef..9231e0b4e4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/privateDM/ChatroomView.kt @@ -54,6 +54,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.RefreshingChatro import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayReachState import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimit import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimitMarkers +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimitSentinels import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.dal.ChatroomFeedViewModel import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.datasource.ChatroomFilterAssemblerSubscription import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.send.ChatNewMessageViewModel @@ -274,15 +275,24 @@ fun ChatroomViewUI( DmHistoryLoadingCard(nip04Name, "NIP-04", loadingNip04, nip04Exhausted, nip04Relays, nip04Reached) } }, - // Each relay's window-limit marker, placed at its reached cursor, doubles as the load - // sentinel that pulls that relay's next page while it's on screen (see - // RelayWindowLimitMarkers). Hidden once both protocols are exhausted. + // Each relay's window-limit marker, placed at its reached cursor (pure UI). Hidden once + // both protocols are exhausted. markersInGap = if (limits.isEmpty()) { null } else { { newer, older -> RelayWindowLimitMarkers(limits, newer, older) } }, + // The hoisted load driver that pulls each relay's next page while its marker is on screen, + // off viewport visibility (see RelayWindowLimitSentinels) so feed reorders don't re-page. + sentinels = + if (limits.isEmpty()) { + null + } else { + { items, listState -> + RelayWindowLimitSentinels(limits, listState) { index -> items.getOrNull(index)?.event?.createdAt } + } + }, ) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/feed/ChatroomListFeedView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/feed/ChatroomListFeedView.kt index e0342ed08a..9189d8b3ab 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/feed/ChatroomListFeedView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/feed/ChatroomListFeedView.kt @@ -54,6 +54,7 @@ import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.DmHistoryLoading import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayReachState import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimit import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimitMarkers +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.feed.layouts.RelayWindowLimitSentinels import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.rooms.ChatroomHeaderCompose import com.vitorpamplona.amethyst.ui.theme.DividerThickness import com.vitorpamplona.amethyst.ui.theme.FeedPadding @@ -200,6 +201,10 @@ private fun FeedLoaded( } } + // Hoisted load driver: pulls each relay's next page off viewport visibility, so feed reorders + // (a live DM bumping a room) no longer re-fire paging. The markers below are pure UI. + RelayWindowLimitSentinels(limits, listState) { index -> items.list.getOrNull(index)?.createdAt() } + LazyColumn( contentPadding = rememberFeedContentPadding(FeedPadding), state = listState,