diff --git a/PLAN.md b/PLAN.md index e9f126b..2e27f13 100644 --- a/PLAN.md +++ b/PLAN.md @@ -628,14 +628,14 @@ dev-updater (Kotlin 2.4.x, CMP 1.11.x, JDK 21). Screens: a group's heading or the bar at its foot — is the end that stays put, and the row grows away from it. The transcript is laid out from the bottom, so a row's bottom edge is anchored for free and the top one - has to be arranged. `toggleAnchored` waits on the row's *size*, which - a scroll cannot change — a version that waited on the top edge instead - mistook the reader's own drag for the row resizing and undid it — and - then reads how far things moved off any row from the pressed one - upwards, since a group taller than the screen disappears under the - bottom edge as it shrinks and cannot report its own move (2026-08-30, - asked for after groups opened upwards and sent their own heading off - the top of the screen). + has to be arranged. The correction lives in the *layout* phase + (`Modifier.holdTopEdge`): the measurement that discovers the row's new + height asks the list to shift by that much, via + `requestScrollToItem`, before anything is drawn. Doing it from an + effect instead means the wrong position is drawn once first, which + reads as a flick and gets worse the faster the screen refreshes + (2026-08-30, asked for after groups opened upwards and sent their own + heading off the top of the screen). - Input bar: text, attach (camera/gallery/file), send — **always enabled**; mid-run sends become steering messages. - Top bar: model chip (tap to change), stop button while running, token diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 0f63906..a5bc392 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -5,7 +5,6 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.PickVisualMediaRequest import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.Image -import androidx.compose.foundation.gestures.scrollBy import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -18,7 +17,6 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.LazyListItemInfo import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape @@ -49,6 +47,7 @@ import androidx.compose.runtime.setValue import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics @@ -63,10 +62,8 @@ import java.util.concurrent.atomic.AtomicReference import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeoutOrNull private const val RECONNECT_DELAY_MS = 1500L @@ -84,13 +81,50 @@ private const val RECONNECT_DELAY_MS = 1500L private const val HISTORY_LOOKAHEAD = 8 /** - * How long to keep holding a row's top edge while the row settles to its new height. + * Which row was asked to hold its top edge, and how tall it was when it last measured. * - * Long enough for the second and third layout passes a row can take to reach its final height, and - * short enough that it is over before the reader could have started scrolling for their own - * reasons. It always runs to the end: there is no way to ask whether more passes are coming. + * Deliberately *not* snapshot state, and that is the point of the whole class. Both fields are + * written from the layout phase; a snapshot write there that composition reads would schedule + * another recomposition, and the correction has to land inside the frame that is already being laid + * out rather than in a later one. Nothing observes these, so nothing needs to. + * + * [key] is cleared by the resize it was set for, so it cannot be spent on an unrelated one. */ -private const val ANCHOR_SETTLE_MS = 500L +private class TopEdgeHold { + var key: Any? = null +} + +/** One row's height between layouts, so a change in it can be noticed. See [holdTopEdge]. */ +private class LastHeight { + var value: Int? = null +} + +/** + * Keeps this row's top edge where it is when the row changes height, if it was asked to. + * + * This runs in the *layout* phase, from the measurement that discovers the new height, and that is + * the whole reason it is a modifier rather than an effect. A correction posted to a coroutine + * arrives a frame or more after the layout it is correcting, so the wrong position is drawn once + * before the right one -- visible as a flick, and worse the faster the screen refreshes. Scrolling + * from here happens before anything is drawn, so there is no frame to see and nothing that depends + * on how quickly the correction is scheduled. + * + * [hold] is given the change in height. The row's bottom edge is held by the list, so a scroll of + * exactly that much is what leaves the top edge where it was. + */ +@Composable +private fun Modifier.holdTopEdge(key: Any, held: TopEdgeHold, hold: (Int) -> Unit): Modifier { + val last = remember { LastHeight() } + return onSizeChanged { size -> + val previous = last.value + last.value = size.height + // A first measurement has no previous height to have moved from, and a row that came + // back after being scrolled away is a first measurement again. + if (previous == null || previous == size.height || held.key != key) return@onSizeChanged + held.key = null + hold(size.height - previous) + } +} /** * What the transcript renders: the event stream folded into displayable rows (see [foldEvent]). The @@ -490,6 +524,7 @@ fun SessionScreen( onSettings: () -> Unit, ) { val scope = rememberCoroutineScope() + val topEdgeHeld = remember { TopEdgeHold() } var items by remember { mutableStateOf(listOf()) } var status by remember { mutableStateOf(summary.status) } // Seeded from the row this screen was opened from, so a conversation already under way says @@ -692,72 +727,13 @@ fun SessionScreen( * up off the screen and fills the space above it, so the calls appear on the far side of the * control that produced them. * - * So [RowEdge.Bottom] is the list's own behaviour and does nothing extra, and [RowEdge.Top] - * scrolls back by however much the row grew. It has to be measured rather than worked out: only - * the layout knows how tall an open group is, and it depends on the calls in it. - * - * What is measured is the row's *size*, and that is the whole reason this works. The obvious - * thing to measure is where its top edge went, and an earlier version did -- but a top edge - * also moves when the reader scrolls, so a correction still waiting for the layout would - * instead wake on the reader's own drag, read their scroll distance as the row's growth, and - * undo it. That was reported as the transcript jumping on every expand and refusing to scroll - * back at all. A size does not change when anybody scrolls, so the two cannot be confused. - * - * The row's bottom edge is held by the list, so scrolling by the growth is what puts the top - * edge back, and it is done for every step of that growth until [ANCHOR_SETTLE_MS] is up. + * So [RowEdge.Bottom] is the list's own behaviour and needs nothing, while [RowEdge.Top] marks + * the row as holding its top edge the next time it is measured. The correction itself belongs + * to the measurement -- see [holdTopEdge]. */ fun toggleAnchored(key: Any, edge: RowEdge, toggle: () -> Unit) { - fun rowOf() = listState.layoutInfo.visibleItemsInfo.firstOrNull { it.key == key } - val row = rowOf() + if (edge == RowEdge.Top) topEdgeHeld.key = key toggle() - // Not on screen when it was pressed, so there is no edge of it to hold. - if (edge == RowEdge.Bottom || row == null) return - // Read out now, as numbers: the layout hands back a reused object per slot, so holding - // one and reading it later describes whatever item took that slot since. - val rowIndex = row.index - val rowSize = row.size - - // How far the row's top edge moved, read off any row that moved with it. The row and - // everything above it shift by exactly the row's growth, so they all give the same - // answer -- and several are recorded because which of them can answer varies. A row - // that shrinks slides down behind its own anchored bottom edge, and a group taller - // than the screen goes under the bottom of it entirely, leaving nothing of itself to - // measure. - fun edgeOf(item: LazyListItemInfo) = - if (item.index == rowIndex) item.offset + item.size else item.offset - val marks = - listState.layoutInfo.visibleItemsInfo - .filter { it.index >= rowIndex } - .associate { it.key to edgeOf(it) } - - scope.launch { - // Waited on by *size*, which is the one thing a scroll cannot change. An earlier - // version waited on the top edge instead, so a correction still pending would wake - // on the reader's own drag, read their scroll distance as the row's growth and undo - // it -- reported as the transcript jumping on every expand and refusing to scroll - // back at all. - // - // The wait answers `true` rather than the size it saw, because the size it saw is - // sometimes `null` -- that is the row leaving the screen, which is a real answer and - // the one this has to handle. Returned straight out of `withTimeoutOrNull` it would - // be the same value that means "never happened", and the case needing the correction - // most was the case silently skipped. - val settled = - withTimeoutOrNull(ANCHOR_SETTLE_MS) { - snapshotFlow { rowOf()?.size }.first { it != rowSize } - true - } - if (settled == null) return@launch - // Never against a finger already on the screen: the reader is placing the list - // themselves, and that is also the only way a mark could have moved for a reason - // other than the row resizing. - if (listState.isScrollInProgress) return@launch - val moved = - listState.layoutInfo.visibleItemsInfo.firstNotNullOfOrNull { item -> - marks[item.key]?.let { edgeOf(item) - it } - } ?: return@launch - listState.scrollBy(moved.toFloat()) - } } // A compaction reports nothing about its own progress -- measured against the CLI, which @@ -1211,107 +1187,132 @@ fun SessionScreen( // at the same end. Paging older history is the opposite insertion and was // already fine, and stays fine, because a key survives both. items(rows.asReversed(), key = { it.key }) { row -> - when (row) { - is TranscriptRow.Tools -> - ToolGroup( - group = row, - expanded = row.id in expandedGroups, - onToggle = { edge -> - toggleAnchored(row.key, edge) { - expandedGroups = - if (row.id in expandedGroups) expandedGroups - row.id - else expandedGroups + row.id - } - }, - isToolExpanded = { it in expandedTools }, - // Anchored on the group, not the call: opening one call makes - // the whole group taller, and the heading the reader is under - // is the group's. - onToolToggle = { id -> - toggleAnchored(row.key, RowEdge.Top) { - expandedTools = - if (id in expandedTools) expandedTools - id - else expandedTools + id - } - }, - onAnswer = { questionId, answers -> - act { - answerQuestion(settings, summary.id, questionId, answers) - } - }, - image = { ref -> SessionImage(settings, summary.id, ref) }, + Box( + Modifier.holdTopEdge(row.key, topEdgeHeld) { grew -> + // Requested rather than scrolled. Scrolling forces a remeasure, + // and forcing one from inside a measure throws; this is the form + // built to be asked for during layout and applied in that pass. + listState.requestScrollToItem( + listState.firstVisibleItemIndex, + listState.firstVisibleItemScrollOffset + grew, ) - is TranscriptRow.Single -> - when (val item = row.item) { - is TranscriptItem.UserMsg -> - UserBubble( - settings = settings, - sessionId = summary.id, - text = item.text, - images = item.images, - ) - is TranscriptItem.AssistantMsg -> AssistantMessage(item.text) - is TranscriptItem.ToolRun -> - ToolCard( - tool = item, - expanded = item.id in expandedTools, - onToggle = { - toggleAnchored(row.key, RowEdge.Top) { - expandedTools = - if (item.id in expandedTools) - expandedTools - item.id - else expandedTools + item.id - } - }, - onAnswer = { questionId, answers -> + } + ) { + when (row) { + is TranscriptRow.Tools -> + ToolGroup( + group = row, + expanded = row.id in expandedGroups, + onToggle = { edge -> + toggleAnchored(row.key, edge) { + expandedGroups = + if (row.id in expandedGroups) + expandedGroups - row.id + else expandedGroups + row.id + } + }, + isToolExpanded = { it in expandedTools }, + // Anchored on the group, not the call: opening one call makes + // the whole group taller, and the heading the reader is under + // is the group's. + onToolToggle = { id -> + toggleAnchored(row.key, RowEdge.Top) { + expandedTools = + if (id in expandedTools) expandedTools - id + else expandedTools + id + } + }, + onAnswer = { questionId, answers -> + act { + answerQuestion( + settings, + summary.id, + questionId, + answers, + ) + } + }, + image = { ref -> SessionImage(settings, summary.id, ref) }, + ) + is TranscriptRow.Single -> + when (val item = row.item) { + is TranscriptItem.UserMsg -> + UserBubble( + settings = settings, + sessionId = summary.id, + text = item.text, + images = item.images, + ) + is TranscriptItem.AssistantMsg -> AssistantMessage(item.text) + is TranscriptItem.ToolRun -> + ToolCard( + tool = item, + expanded = item.id in expandedTools, + onToggle = { + toggleAnchored(row.key, RowEdge.Top) { + expandedTools = + if (item.id in expandedTools) + expandedTools - item.id + else expandedTools + item.id + } + }, + onAnswer = { questionId, answers -> + act { + answerQuestion( + settings, + summary.id, + questionId, + answers, + ) + } + }, + image = { ref -> + SessionImage(settings, summary.id, ref) + }, + ) + is TranscriptItem.QuestionCard -> + QuestionRow(item) { answers -> act { answerQuestion( settings, summary.id, - questionId, + item.id, answers, ) } - }, - image = { ref -> SessionImage(settings, summary.id, ref) }, - ) - is TranscriptItem.QuestionCard -> - QuestionRow(item) { answers -> - act { - answerQuestion(settings, summary.id, item.id, answers) } - } - is TranscriptItem.ErrorMsg -> - Text( - item.message, - color = MaterialTheme.colorScheme.error, - style = MaterialTheme.typography.bodyMedium, - ) - is TranscriptItem.ImageItem -> - SessionImage(settings, summary.id, item.ref) - is TranscriptItem.Note -> - Text( - item.text, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - is TranscriptItem.CommandRow -> CommandBubble(item.text) - is TranscriptItem.ClearedNote -> ClearedRow() - is TranscriptItem.CompactedNote -> CompactedRow(item) - is TranscriptItem.PeerNote -> - PeerMessageRow( - item = item, - expanded = item.seq in expandedNotes, - onToggle = { - toggleAnchored(row.key, RowEdge.Top) { - expandedNotes = - if (item.seq in expandedNotes) - expandedNotes - item.seq - else expandedNotes + item.seq - } - }, - ) - } + is TranscriptItem.ErrorMsg -> + Text( + item.message, + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodyMedium, + ) + is TranscriptItem.ImageItem -> + SessionImage(settings, summary.id, item.ref) + is TranscriptItem.Note -> + Text( + item.text, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + is TranscriptItem.CommandRow -> CommandBubble(item.text) + is TranscriptItem.ClearedNote -> ClearedRow() + is TranscriptItem.CompactedNote -> CompactedRow(item) + is TranscriptItem.PeerNote -> + PeerMessageRow( + item = item, + expanded = item.seq in expandedNotes, + onToggle = { + toggleAnchored(row.key, RowEdge.Top) { + expandedNotes = + if (item.seq in expandedNotes) + expandedNotes - item.seq + else expandedNotes + item.seq + } + }, + ) + } + } } } }