fix(chat): serialize scroll follow and timeline jumps (#1322, #1323) (#1359)

This commit is contained in:
m57 authored and GitHub committed 2026-09-28 15:59:29 +03:00
1 parent 58794bee00
commit cc98f3ba89
5 files changed
+142 -58

No files matched your search

@@ -3,6 +3,8 @@ package com.m57.hermescontrol.ui.chat.fullbleed
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.width
import androidx.compose.foundation.lazy.LazyListState
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertCountEquals
@@ -12,6 +14,9 @@ import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onAllNodesWithTag
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.onRoot
import androidx.compose.ui.test.performTouchInput
import androidx.compose.ui.test.swipeDown
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.MediumTest
@@ -25,6 +30,7 @@ import com.m57.hermescontrol.ui.chat.ToolStatus
import com.m57.hermescontrol.ui.chat.components.ChatScrollController
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import org.junit.Assert.assertFalse
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
@@ -72,9 +78,18 @@ class FullBleedChatListTest {
showUserMessageTokens: Boolean = true,
showAssistantMessageTokens: Boolean = true,
showTokensPerSecond: Boolean = true,
onController: (ChatScrollController) -> Unit = {},
) {
composeTestRule.setContent {
val listState = LazyListState()
val listState = remember { LazyListState() }
val scrollController =
remember(listState) {
ChatScrollController(
listState = listState,
scope = CoroutineScope(Dispatchers.Main.immediate),
)
}
SideEffect { onController(scrollController) }
FullBleedChatList(
transcript =
TranscriptUiState.resolve(
@@ -106,15 +121,24 @@ class FullBleedChatListTest {
actions = testTranscriptActions(),
searchState = ChatSearchState(),
listState = listState,
scrollController =
ChatScrollController(
listState = listState,
scope = CoroutineScope(Dispatchers.Main.immediate),
),
scrollController = scrollController,
)
}
}
@Test
fun upwardUserGesture_pausesBottomFollow() {
lateinit var scrollController: ChatScrollController
render(
messages = List(40) { index -> msg("u$index", MessageRole.USER) },
onController = { scrollController = it },
)
composeTestRule.runOnIdle { scrollController.jumpToBottom() }
composeTestRule.waitForIdle()
composeTestRule.onRoot().performTouchInput { swipeDown() }
composeTestRule.runOnIdle { assertFalse(scrollController.isFollowingBottom) }
}
@Test
fun userMessages_keepBubble_agentProse_isFullBleed() {
render(
@@ -329,8 +329,7 @@ fun ChatScreen(
}
LaunchedEffect(timelineState.historyAnchorRowId) {
if (timelineState.isHistorical && displayedMessages.isNotEmpty()) {
scrollController.pauseFollowing()
listState.scrollToItem(0)
scrollController.jumpToHistoryStart()
}
}
val showScrollToBottom by remember(timelineState.isHistorical, displayedMessages) {
@@ -10,10 +10,9 @@ import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshotFlow
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeoutOrNull
import kotlinx.coroutines.yield
/**
@@ -60,14 +59,8 @@ class ChatScrollController(
private var lastTailKey: Any? = null
private var lastMessageCount: Int = 0
/**
* Guard flag: true while a programmatic scroll-to-bottom is in flight.
* Prevents [observeUserScrollPosition] from flipping [isFollowingBottom]
* to false during the layout race — the scroll targets the old last item,
* Compose lays out the new item below, and [isAtBottom] momentarily
* returns false before the second scroll pass catches it.
*/
private var isProgrammaticScroll by mutableStateOf(false)
/** The sole active scroll command. Replaced by newer tail, search or explicit jumps. */
private var scrollJob: Job? = null
/**
* Pixel tolerance for "at bottom" detection. Covers the LazyColumn's
@@ -77,35 +70,31 @@ class ChatScrollController(
*/
var bottomPixelTolerance by mutableStateOf(48)
/** Serialized scroll scope — all scroll jobs funnel through here. */
fun launchScroll(block: suspend CoroutineScope.() -> Unit) {
scope.launch(block = block)
/** Replace the previous scroll command; user gestures can cancel the same job. */
private fun launchScroll(block: suspend CoroutineScope.() -> Unit) {
scrollJob?.cancel()
scrollJob = scope.launch(block = block)
}
/**
* Read the live bottom-follow state from [LazyListState] and react to
* user-driven scroll position changes. Should be called inside a
* `LaunchedEffect(Unit)` so it observes the list for the screen's lifetime.
*/
/** Observe only arrival at the bottom; departures are owned by actual user input. */
fun observeUserScrollPosition() {
scope.launch {
var previouslyAtBottom = listState.isAtBottom(bottomPixelTolerance)
snapshotFlow { listState.isAtBottom(bottomPixelTolerance) }
.distinctUntilChanged()
.collect { atBottom ->
if (atBottom) {
if (atBottom && !previouslyAtBottom) {
isFollowingBottom = true
// Reaching the bottom means the reader caught up.
pendingCount = 0
} else if (!isProgrammaticScroll) {
// Only break follow on USER-initiated scrolls.
// During programmatic scrolls the viewport is
// transiently not-at-bottom while new items lay out.
isFollowingBottom = false
}
previouslyAtBottom = atBottom
}
}
}
/** An upward user gesture takes ownership from auto-follow immediately. */
fun onUserScrollUp() = pauseFollowing()
/**
* Call on every tail-content change (new message, streaming token, thinking
* toggle, subagent card, clarify prompt). [tailKey] must be a stable value
@@ -132,14 +121,7 @@ class ChatScrollController(
// Pin instantly so rapid streamed tokens don't queue competing
// animations; the reader stays glued to the growing tail.
// Use layout-aware scrolling to survive the Compose layout race.
scope.launch {
isProgrammaticScroll = true
try {
scrollToBottomAwaitingLayout()
} finally {
isProgrammaticScroll = false
}
}
launchScroll { scrollToBottomAwaitingLayout() }
} else {
pendingCount += newMessages
}
@@ -163,14 +145,10 @@ class ChatScrollController(
yield()
yield()
listState.scrollToBottom(animated = animated)
// If we're still not at the bottom after the first scroll (e.g. a
// new item was laid out between the scroll and now), do one more pass.
// Yield once more if layout changed during the first pass, then align
// the current last row; newer tails replace this job instead of racing it.
if (!listState.isAtBottom(bottomPixelTolerance)) {
// Wait up to 150ms for the layout to settle with the new items.
withTimeoutOrNull(150L) {
snapshotFlow { listState.layoutInfo.totalItemsCount }
.first { it > 0 }
}
yield()
listState.scrollToBottom(animated = animated)
}
}
@@ -179,19 +157,19 @@ class ChatScrollController(
fun jumpToBottom(animated: Boolean = false) {
pendingCount = 0
isFollowingBottom = true
scope.launch {
isProgrammaticScroll = true
try {
scrollToBottomAwaitingLayout(animated = animated)
} finally {
isProgrammaticScroll = false
}
}
launchScroll { scrollToBottomAwaitingLayout(animated = animated) }
}
/** An explicit history gesture must not be undone by a short list's bottom-follow. */
fun pauseFollowing() {
isFollowingBottom = false
scrollJob?.cancel()
}
/** Historical timeline navigation shares the scroll command with follow and search. */
fun jumpToHistoryStart() {
pauseFollowing()
launchScroll { listState.scrollToItem(0) }
}
/** FAB tap: resume following + clear unread. */
@@ -218,10 +196,11 @@ class ChatScrollController(
contentOffset: Int,
contentLength: Int,
) {
scope.launch {
pauseFollowing()
launchScroll {
listState.animateScrollToItem(index)
val info = listState.layoutInfo
val item = info.visibleItemsInfo.firstOrNull { it.index == index } ?: return@launch
val item = info.visibleItemsInfo.firstOrNull { it.index == index } ?: return@launchScroll
val viewportHeight = info.viewportEndOffset - info.viewportStartOffset
val fraction = if (contentLength > 0) contentOffset.toFloat() / contentLength else 0f
val targetTop = (viewportHeight / 3) - (item.size * fraction).toInt()
@@ -237,6 +237,9 @@ fun FullBleedChatList(
available: Offset,
source: NestedScrollSource,
): Offset {
if (source == NestedScrollSource.UserInput && available.y > 0f) {
scrollController.onUserScrollUp()
}
if (prefetch.onScroll(
firstVisibleIndex = listState.firstVisibleItemIndex,
deltaY = available.y,
@@ -3,16 +3,22 @@ package com.m57.hermescontrol.ui.chat.components
import androidx.compose.foundation.lazy.LazyListItemInfo
import androidx.compose.foundation.lazy.LazyListLayoutInfo
import androidx.compose.foundation.lazy.LazyListState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.Snapshot
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import org.junit.After
@@ -151,6 +157,79 @@ class ChatScrollControllerTest {
coVerify(exactly = 0) { listState.animateScrollToItem(any(), any()) }
}
@Test
fun `layout growth does not pause follow but returning to bottom after user gesture resumes`() =
runTest(testDispatcher) {
var atBottom by mutableStateOf(true)
val listState = mockLazyListStateAtBottom()
val layoutInfo = listState.layoutInfo
val itemInfo = layoutInfo.visibleItemsInfo.single()
every { itemInfo.index } answers { if (atBottom) 4 else 3 }
val controller = ChatScrollController(listState, backgroundScope)
controller.observeUserScrollPosition()
runCurrent()
atBottom = false // A new row laid out below the viewport, not a user scroll.
Snapshot.sendApplyNotifications()
runCurrent()
assertTrue(controller.isFollowingBottom)
controller.onUserScrollUp()
assertFalse(controller.isFollowingBottom)
atBottom = true
Snapshot.sendApplyNotifications()
runCurrent()
assertTrue(controller.isFollowingBottom)
}
@Test
fun `user upward gesture cancels in-flight follow and counts new messages`() =
runTest(testDispatcher) {
val listState = mockLazyListStateAtBottom()
coEvery { listState.scrollToItem(4, any()) } coAnswers { delay(1_000) }
val controller = ChatScrollController(listState, this)
controller.onTailChanged(tailKey = "first", messageCount = 1)
runCurrent()
controller.onUserScrollUp()
controller.onTailChanged(tailKey = "second", messageCount = 2)
advanceUntilIdle()
assertFalse(controller.isFollowingBottom)
assertEquals(1, controller.pendingCount)
coVerify(exactly = 1) { listState.scrollToItem(4, any()) }
}
@Test
fun `historical jump replaces an in-flight follow and leaves follow paused`() =
runTest(testDispatcher) {
val listState = mockLazyListStateAtBottom()
coEvery { listState.scrollToItem(4, any()) } coAnswers { delay(1_000) }
val controller = ChatScrollController(listState, this)
controller.onTailChanged(tailKey = "tail", messageCount = 1)
runCurrent()
controller.jumpToHistoryStart()
advanceUntilIdle()
assertFalse(controller.isFollowingBottom)
coVerify(exactly = 1) { listState.scrollToItem(0, any()) }
}
@Test
fun `rapid tails replace older jobs rather than queueing scrolls`() =
runTest(testDispatcher) {
val listState = mockLazyListStateAtBottom()
val controller = ChatScrollController(listState, this)
controller.onTailChanged(tailKey = "one", messageCount = 1)
controller.onTailChanged(tailKey = "two", messageCount = 2)
advanceUntilIdle()
assertTrue(controller.isFollowingBottom)
coVerify(exactly = 1) { listState.scrollToItem(4, any()) }
}
@Test
fun `showFab returns true only when paused and content is present`() {
val listState = mockk<LazyListState>(relaxed = true)