diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/LongReply.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/LongReply.kt new file mode 100644 index 0000000..343b0aa --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/LongReply.kt @@ -0,0 +1,101 @@ +package com.example.aiapp + +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier + +/** + * How much of a reply is drawn before the reader is offered the rest. + * + * A limit on the *source*, not on the height, and that is the whole point. Clipping a laid-out row + * to a height saves nothing: Compose measures the text and then throws the overflow away, so the + * line-breaking has already happened. Cutting the string before it is parsed is what stops the work + * from being done -- and it stops the parse too, which the height version could never reach. + * + * Four thousand characters is about two screenfuls of body text on a phone. Two rather than one so + * that a reply somewhat over the limit is not cut nearly in half, and so a reader who does not + * press anything still gets more than they can see at once. + * + * The measurement that made this worth having, from the ai-app-2 session on 2026-08-30: one message + * in a real transcript is over 14,000px tall -- seven screens -- and a single frame spent 59.6ms in + * `measureAndLayout` when it entered the viewport. That cost is proportional to the whole row + * however little of it is on screen, and it is paid again every time the row comes back. + */ +private const val REPLY_CAP_CHARS = 4000 + +/** + * How far past the cap a reply has to be before it is worth cutting. + * + * Without this a message of 4,001 characters loses one character and gains a button, which is worth + * nothing to anybody and costs a control that has to be read and decided about. The row that needs + * this treatment is several times the limit, not just over it. + */ +private const val REPLY_CAP_SLACK = 1000 + +/** + * The opening of [text] if it is long enough to be worth cutting, or null if it should be drawn + * whole. + * + * Cut at a line ending, because a markdown source cut mid-line is a different document: half a + * heading marker, a list item with no bullet, a link whose closing bracket is in the part that was + * dropped. A whole number of lines is the coarsest cut that cannot invent syntax. + * + * A fence left open by the cut is closed, which is the one case a line boundary does not save. An + * unterminated ``` swallows the rest of the reply into a code block, so the truncation would change + * how the part still on screen is *drawn* rather than only how much of it there is -- and a reader + * has no way to tell that from the reply genuinely having been code. + */ +fun shortenedReply(text: String, limit: Int = REPLY_CAP_CHARS): String? { + if (text.length <= limit + REPLY_CAP_SLACK) return null + val cut = text.lastIndexOf('\n', limit).let { if (it <= 0) limit else it } + val head = text.substring(0, cut) + // Fences are counted rather than matched: an opening and a closing one are the same token, so + // an odd number of them is an opening that never closed. + return if (head.split("\n").count { it.trimStart().startsWith("```") } % 2 == 1) "$head\n```" + else head +} + +/** + * A reply drawn to [REPLY_CAP_CHARS], with the rest a press away. + * + * The control says how much is behind it rather than only "Show more", because the two answers a + * reader wants are different sizes of decision: another paragraph is worth opening while standing + * in a scroll, and another twenty screens is worth knowing about first. + * + * Expanded is remembered by the screen rather than by this row, so scrolling away and back does not + * shut something the reader deliberately opened -- see SessionScreen's other expansion sets. + */ +@Composable +fun CappedReply( + full: String, + shortened: String, + replies: ParsedReplies, + onShowMore: () -> Unit, + modifier: Modifier = Modifier, +) { + Column(modifier.fillMaxWidth()) { + AssistantMessage(shortened, replies) + TextButton(onClick = onShowMore) { + Text( + "Show the rest (${remaining(full.length - shortened.length)})", + style = MaterialTheme.typography.labelLarge, + ) + } + } +} + +/** What is left, in the units a reader thinks in rather than in characters. */ +private fun remaining(chars: Int): String { + // Against the same figure the cap is written in, so the two cannot drift: a "screenful" here is + // whatever [REPLY_CAP_CHARS] is two of. + val screens = chars.toDouble() / (REPLY_CAP_CHARS / 2) + return when { + screens < 1.5 -> "about another screen" + screens < 20 -> "about ${Math.round(screens)} more screens" + else -> "more than 20 screens" + } +} diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt index 70ac150..51f6057 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt @@ -57,8 +57,18 @@ private fun partsOf(text: String): List { return if (parts.singleOrNull() is MessagePart.Prose) listOf(MessagePart.Prose(text)) else parts } -/** Every string a reply will be drawn from, for [ParsedReplies.warm] to make ready. */ -fun markdownIn(text: String): List = partsOf(text).map { it.text } +/** + * Every string a reply will be drawn from, for [ParsedReplies.warm] to make ready. + * + * Both forms of a long one, because which gets drawn is not decided here: the newest reply is drawn + * whole and every other long one is drawn cut (see [shortenedReply]), and the reader can ask for + * the rest of any of them. Warming only one of the two would leave the other parsing on the thread + * that draws, in the frame the row appears -- and a string warmed under a key no row ever looks up + * is a miss that nothing reports. The extra parse is off the composing thread, which is the only + * place it would have cost anything. + */ +fun markdownIn(text: String): List = + partsOf(text).flatMap { part -> listOfNotNull(part.text, shortenedReply(part.text)) } @Composable private fun MemoryNote(note: MessagePart.Remembered, replies: ParsedReplies) { 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 61630f5..bf56da4 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -7,8 +7,8 @@ import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.Image import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize @@ -17,9 +17,6 @@ import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape import androidx.compose.material3.AlertDialog import androidx.compose.material3.Button @@ -48,6 +45,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.draw.drawWithContent import androidx.compose.ui.layout.onGloballyPositioned import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.layout.positionInRoot @@ -84,11 +82,13 @@ private val LOADING_SPINNER = 48.dp * How much history to keep loaded past the oldest row on screen, counted in screenfuls. * * Both the point at which history starts loading and how much of it a load has to produce before it - * stops. Measured against what is on screen rather than written down as a number of rows, because a - * row is anything from one line to a page and a fixed count is therefore a distance only by - * accident. Eight rows was the number, and on a tool-heavy transcript eight rows is less than one - * screen: the reader reached the end of what was loaded on *every* swipe and waited a round trip - * standing there, which is a list running out of transcript rather than a slow frame. + * stops. Multiplied by the viewport to give a number of *pixels* of scroll, which is the distance + * the question is actually about: how far the reader can keep going before they run out. A row is + * anything from one line to a page, so a count of rows is that distance only by accident. Eight + * rows was the number, and on a tool-heavy transcript eight rows is less than one screen: the + * reader reached the end of what was loaded on *every* swipe and waited a round trip standing + * there, which is a list running out of transcript rather than a slow frame. Counting screenfuls of + * rows fixed the size of the mistake without fixing its kind; pixels are the unit itself. * * Three, so a fling lands on rows that are already there and the page after them is on its way. The * cost of being generous is a page fetched that nobody reads; the cost of being mean is a list that @@ -644,6 +644,10 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // by default, which is the rule for anything new in this transcript: a screen that opens // everything it can is one nobody can scan. var expandedNotes by remember { mutableStateOf(setOf()) } + // Long replies the reader has asked to see the rest of, by the seq of the row. Held here + // rather than in the row so that scrolling away and back does not shut something they + // deliberately opened -- the same reason the sets above it are here. + var expandedReplies by remember { mutableStateOf(setOf()) } // Uploaded-but-not-yet-sent attachment ids; sent with the next message. var pendingAttachments by remember { mutableStateOf(listOf()) } // What this session is set to now, seeded from the row that opened it and @@ -671,21 +675,12 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // it is the question "where did I leave off" and the answer stops being interesting the // moment the list is on screen. val savedAnchor = remember(summary.id) { loadScrollAnchor(context, summary.id) } - // Whether the list is still being put back where it was left. Nothing is drawn while it is: - // opening at the newest end and then travelling to the anchor is exactly the journey - // `reverseLayout` exists to remove, and this transcript is not allowed to move under a reader. + // Whether the history the saved position needs is still being fetched. Nothing is drawn while + // it is: opening at the newest end and then travelling to the anchor is exactly the journey + // this layout exists to remove, and this transcript is not allowed to move under a reader. + // The other half of the wait is [TranscriptScroll.settling], which covers the frames between + // the rows arriving and the layout that measures them putting the position back. var restoring by remember(summary.id) { mutableStateOf(savedAnchor != null) } - // Remembered, and only ever written when a scroll settles -- so it records where the reader - // last left the list, and an insertion cannot change the answer. Reading the live position - // instead looks right and is subtly wrong: a keyed list moves its anchor to keep the reader's - // content still, so by the time the new item can be observed the view is already one item - // away from the newest and reports itself as scrolled back. The message then never followed, - // which was visible as a compaction whose progress bar sat just off the bottom of the screen - // while the button that started it said it was running. - // - // Seeded from whether there is a position to go back to, so the correction it drives does not - // pull the list to the newest end before the restore has put it anywhere. - var followTail by remember(summary.id) { mutableStateOf(savedAnchor == null) } // Sent, but not yet read by the session -- which is when the backend // records it and it comes back as a row. Until then it is drawn below // the working indicator, because that is where it is in the session's @@ -707,23 +702,25 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // Replies parsed ahead of the rows that draw them; see [ParsedReplies]. Per session, because // it describes that session's rows and nothing else. val replies = remember(summary.id) { ParsedReplies() } - val listState = rememberLazyListState() - // Whether the newest message is on screen right now. The list is laid out from the bottom - // (see the LazyColumn below), so "newest" is index 0 and being there is being at the start of - // it. This is what the jump-to-newest button watches: it is about what the reader can see. + val listState = rememberTranscriptScroll(summary.id) + // Whether the newest message is on screen right now. The content hangs from its newest end + // (see [TranscriptScroll]), so being there is being at scroll position zero. This is what the + // jump-to-newest button watches: it is about what the reader can see. // // It is also the gate on everything the list draws -- see [record]. - val atNewest by remember { - derivedStateOf { - listState.firstVisibleItemIndex == 0 && listState.firstVisibleItemScrollOffset == 0 - } - } + val atNewest by remember { derivedStateOf { listState.atNewest } } // Transcript events that arrived while somebody was reading further back, in the order they // arrived, waiting for them to return to the newest end. See [record] for why. var held by remember { mutableStateOf(listOf()) } // What is actually drawn: the transcript with runs of adjacent tool // calls folded into one row each. val rows = remember(items) { groupToolRuns(items) } + // The reply drawn whole however long it is; see the transcript list below. Recomputed with + // `items` rather than tracked as it arrives, because "newest" moves: a reply that was the + // last one becomes history the moment the next turn starts, and a row that kept its + // exemption after that would be the one enormous row this exists to bound. + val newestReply = + remember(items) { items.filterIsInstance().lastOrNull()?.seq } /** * Everything the transcript list draws, from one event. @@ -843,46 +840,26 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } /** - * How many list items sit above every transcript row -- one while something is waiting, none - * otherwise. + * Whether the row holding transcript position [seq] is loaded, with older history behind it. * - * Read both here and by the `item` that draws that bubble, so a restored position and the list - * cannot disagree about what is at which index. Anything else added above the rows later - * belongs in this count. - */ - fun itemsAboveRows() = if (queued.isNotEmpty() || waitingCommands.isNotEmpty()) 1 else 0 - - /** Everything the list draws, in items rather than in rows. See [itemsAboveRows]. */ - fun listItemCount() = groupToolRuns(items).size + itemsAboveRows() - - /** - * The row drawn at list index [index], or null when that item is not a row. + * "Behind it" is the part that is easy to leave out. The oldest loaded row is a half-row -- + * [joinPages] welds the other half onto it when the page before it arrives, and it grows -- so + * putting the reader inside one leaves them where they were only until the next page lands, + * which was a screen and a half out. Any row that is not the oldest is final. * - * Computed from `items` rather than from `rows` for the reason [loadOlderPage] gives, and this - * is the caller that makes it matter: the scroll listener below is started once and keyed on - * the list state, so a `rows` read inside it is the value from the *first* composition, which - * is empty. That saved a null anchor on every scroll -- indistinguishable from having been left - * at the newest end, so the position was silently never recorded at all. - */ - fun rowAt(index: Int): TranscriptRow? = - groupToolRuns(items).asReversed().getOrNull(index - itemsAboveRows()) - - /** - * Where the row holding transcript position [seq] sits in the list, or null when nothing loaded - * reaches back that far. + * The last row starting at or before [seq], rather than one starting exactly there: the events + * behind a row can be regrouped between the save and the reopen -- a run of calls folds + * differently when a page boundary moves, and two halves of a reply become one message -- and + * the reader's place is inside whichever row now holds that seq, not gone. * - * Computed from `items` rather than from `rows`, as [rowAt] is and for the same reason. + * Computed from `items` rather than from `rows` for the reason [loadOlderPage] gives: `rows` is + * the composition's value and does not change under a running coroutine. */ - fun indexOfSeq(seq: Long): Int? { - // The last row that starts at or before it, rather than one that starts exactly there: - // the events behind a row can be regrouped between the save and the reopen -- a run of - // calls folds differently when a page boundary moves, and two halves of a reply become - // one message -- and the reader's place is inside whichever row now holds that seq, not - // gone. Rows are oldest-first here, so that is the last match. + fun anchorRow(seq: Long): Long? { val ordered = groupToolRuns(items) - val row = ordered.indexOfLast { it.startSeq <= seq } - if (row < 0) return null - return ordered.size - 1 - row + itemsAboveRows() + val at = ordered.indexOfLast { it.startSeq <= seq } + // Zero is the oldest loaded row, which is the half-row above; not found is -1. + return if (at > 0) ordered[at].startSeq else null } /** @@ -1012,8 +989,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // that -- a tool run is renamed whenever the newest page starts somewhere new, // so an anchor on one was never found and this paged to the first event of the // conversation every time an active session was reopened. - var index = indexOfSeq(anchor.seq) - while (moreHistory && (index == null || index >= listItemCount() - 1)) { + while (moreHistory && anchorRow(anchor.seq) == null) { // The whole span in one request rather than a page at a time. `read_window` // counts *lines* and a transcript numbers them one per seq, so the distance // back to the anchor is the number of events to ask for -- and were seqs ever @@ -1030,17 +1006,25 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // the newest end has to be there for the list to be able to count to it. val span = oldestSeq - anchor.seq + HISTORY_PAGE if (!loadOlderPage(span.coerceIn(1L, RESTORE_PAGE_MAX.toLong()).toInt())) break - index = indexOfSeq(anchor.seq) } - // Both writes before this coroutine yields, so the list's first measurement is - // the one with every row in it *and* the requested position -- the list is drawn - // where it was left rather than drawn and then moved. `requestScrollToItem` is - // the form that is applied during a layout pass; see [holdTopEdge]. + // Both writes before this coroutine yields, so the layout that first measures + // these rows is also the one that puts the position back -- the transcript is + // drawn where it was left rather than drawn and then moved. The pixels do not + // exist until that measurement, which is why the anchor is handed to the layout + // rather than applied here; see [TranscriptScroll.pending]. restoring = false - // A null index is a row that is no longer in the transcript -- a reset stream, or + // Resolved to the row that *holds* the saved position rather than passed + // straight through, because the two are not always the same seq: the events + // behind a row regroup between the save and the reopen -- a run of calls folds + // differently when a page boundary moves, two halves of a reply become one + // message -- and the layout can only recognise a row by the seq it now starts + // at. Handing it the saved seq meant the row it named no longer existed, so the + // position was never applied and the transcript opened at the newest end. + // + // Null is a row that is no longer in the transcript at all -- a reset stream, or // a session cleared from elsewhere. - index?.let { - listState.requestScrollToItem(it, anchor.offset) + anchorRow(anchor.seq)?.let { rowSeq -> + listState.restore(ScrollAnchor(rowSeq, anchor.offset)) restored = true } } @@ -1049,7 +1033,8 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // slow but complete. Saying so beats silently showing nothing. streamError = e.message } - if (!restored) followTail = true + // Nothing to put back, so draw where the content already hangs: the newest end. + if (!restored) listState.giveUp() // Whatever happened above, including a page that never arrived: an empty transcript is a // state the screen can draw, and a permanently blank one is not. restoring = false @@ -1145,18 +1130,6 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () backlog.forEach { record(it) } } } - // Whether they *chose* to be at the newest end, which is a different question from being - // there and the one that decides whether an arriving message brings the view with it. - LaunchedEffect(listState) { - snapshotFlow { listState.isScrollInProgress } - // The value `snapshotFlow` emits on collection is the state of things before anybody - // has touched the list, and it is `false` -- which reads here as a scroll that has - // just ended, and so as an answer about where the reader chose to be. Only the - // transitions after it are scrolls. - .drop(1) - .collect { scrolling -> if (!scrolling) followTail = atNewest } - } - // Where the reader left off, written whenever the list settles somewhere new. // // Driven by the position rather than by the scroll flag, and that is the whole point: a @@ -1165,115 +1138,63 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // that, and it left the old position recorded -- so the reader pressed the control that means // "take me to the end", left, came back, and was put back where they had been. // - // Deliberately not where [followTail] is decided, which has to stay on the settle: a keyed - // list moves its own anchor to keep content still when a row arrives, so the position reports - // itself as one item back for a frame every time a message lands. That frame is the whole - // reason [followTail] is a remembered answer, and reading it here instead would stop new - // messages being followed. + // There is no separate "did they choose to be here" flag any more, and there is nothing left + // for one to protect against. It existed because a keyed lazy list moved its own anchor when a + // row arrived, so for one frame the position reported itself scrolled back from the newest end + // when nobody had scrolled at all. This layout hangs from that end, so a row arriving does not + // move the position: zero still means the newest message, during the frame it lands and after. LaunchedEffect(listState) { - snapshotFlow { - if (listState.isScrollInProgress) null - else listState.firstVisibleItemIndex to listState.firstVisibleItemScrollOffset - } - // As above: where the list sits before anybody has touched it is not somewhere they - // left off, and taking it as one wiped every saved anchor on the way in -- before the - // restore below could use it. + snapshotFlow { if (listState.scroll.isScrollInProgress) null else listState.scroll.value } + // The value `snapshotFlow` emits on collection is where the list sits before anybody + // has touched it, which is not somewhere they left off. Taking it as one wiped every + // saved anchor on the way in -- before the restore above could use it. .drop(1) .collect { settled -> - if (settled == null) return@collect + if (settled == null || listState.settling) return@collect saveScrollAnchor( context, summary.id, // Nothing to restore at the newest end, which is where a session with no // anchor opens anyway -- so the ordinary case costs a `remove` and no - // page-back on the way in. [followTail] covers the frame described above, - // where a row has just arrived and the position has not caught up yet; - // `rowAt` is null for the pending bubble, which sits above every row and is - // not one, and being on that is being at the newest end too. - if (followTail || atNewest) null - else rowAt(settled.first)?.let { ScrollAnchor(it.startSeq, settled.second) }, + // page-back on the way in. + if (settled == 0) null else listState.anchor(), ) } } - // A new item at the newest end shifts every index by one, so the view - // has to step back to 0 to stay put. One item, instantly -- not a - // journey through the transcript. - // Anything that changes how much room the list has, as well as a new - // item arriving. Typing is the case that gets missed: the field grows - // from one line to four and the keyboard opens under it, and neither - // is a new message, so watching the item count alone leaves the newest - // text drifting out of sight while somebody writes a reply to it. + // Reaching the far end of what is loaded fetches the page before it. // - // Counted in list items rather than in transcript rows, because the rows are not all of it: - // the working indicator and a queued message are items too, and they arrive at exactly the - // same end. Sibling to the paging trigger below, which is the same count read from the other - // end for the same reason. + // Measured in pixels of scroll, which is the unit the question is actually about: how far can + // the reader keep going before they run out. Rows are the wrong unit for it and were the + // reason this went wrong twice -- a row is anything from one line to a screenful, so a cushion + // of "three rows" is a different amount of reading depending which three, and a page of eight + // hundred *events* can fold into almost no new rows at all when it is one streamed reply and a + // run of tool calls. Nothing then asked for the next page, and the transcript only loaded when + // somebody dragged it again. // - // Never while a scroll is running, and that is a rule of its own rather than a refinement of - // the condition beside it: a list must not be moved out from under a hand that is moving it. - // The two disagree because [followTail] is deliberately a *remembered* answer, rewritten only - // when a scroll settles -- so for the whole of a fling it still reports the newest end, where - // the reader was when they threw it. A page of history landing during that fling is a change - // in the count, and the correction meant for an insertion at the newest end then fired for - // one at the oldest: the reader was thrown back to the bottom mid-flight. It could happen - // only once, which is what made it look arbitrary rather than mechanical -- the snap settles - // the scroll at the newest end, so the next fling gets far enough to settle away from it, and - // from then on [followTail] is false and nothing fires. Skipping the correction outright is - // right rather than merely safe: the count can only have grown at the newest end while the - // reader is already there, because [record] holds everything else until they come back. - LaunchedEffect(listState) { - snapshotFlow { - Pair(listState.layoutInfo.totalItemsCount, listState.layoutInfo.viewportSize.height) - } - .collect { (count, _) -> - if (followTail && !listState.isScrollInProgress && count > 0) { - listState.scrollToItem(0) - } - } - } - // Reaching the far end of what is loaded -- the oldest item, which in - // this layout is the last index -- fetches the page before it. - // - // Both numbers come from the list itself, and that is the point: an index into what is drawn - // can only be compared against how much is drawn. Three things already make that differ from - // the event count -- a run of adjacent tool calls is one row, and the queued bubble and the - // working indicator are rows with no event behind them at all -- so measuring the far end in - // events meant the threshold could not be reached, and a session with tool calls in it simply - // stopped scrolling back. Anything added to this list later is a fourth, and totalItemsCount - // already counts it. - LaunchedEffect(listState, rows.size, moreHistory) { - snapshotFlow { - val layout = listState.layoutInfo - // How many rows are on screen is the third thing this needs, and it is what turns a - // cushion measured in rows into one measured in screens; see [HISTORY_SCREENS]. - Triple( - layout.visibleItemsInfo.lastOrNull()?.index ?: 0, - layout.totalItemsCount, - layout.visibleItemsInfo.size, - ) - } - .collect { (last, total, onScreen) -> - if (!moreHistory || loadingHistory || total == 0) return@collect - val cushion = onScreen.coerceAtLeast(1) * HISTORY_SCREENS - if (last < total - cushion) return@collect + // There is no correction beside this one any more. Following the newest message used to be an + // effect here too, watching the item count and the viewport for a change and snapping back -- + // and it had to be told not to fire during a scroll, because a page of history landing mid- + // fling looked exactly like a message arriving and threw the reader to the bottom. The content + // now hangs from the newest end, so an arriving message needs no correction and a page of + // history moves nothing; see [TranscriptScroll]. + LaunchedEffect(listState, moreHistory) { + snapshotFlow { listState.roomAbove to listState.viewport } + .collect { (room, viewport) -> + if (!moreHistory || loadingHistory || viewport == 0) return@collect + val cushion = viewport * HISTORY_SCREENS + if (room >= cushion) return@collect loadingHistory = true try { - // Pages until there are rows behind them again, not one page and stop. + // One page, and then this fires again if it was not enough. // - // A page is eight hundred *events*, and events are not rows: a reply arrives - // as hundreds of text deltas that fold into a single message, and a run of - // thirty-five tool calls is one row. So a page that lands can leave the far - // end almost exactly where it was -- and since this is triggered by the far - // end moving, nothing asks for the next one. The list then only loads when - // somebody drags it again, a page at a time, which is what "it only loads - // when you touch the top" was, and later what the wait at every swipe was. - // Counted from `items` rather than from `rows`, which is the - // composition's value and does not change under a running coroutine. - val start = groupToolRuns(items).size - var have = start - while (moreHistory && have - start < cushion && loadOlderPage()) { - have = groupToolRuns(items).size - } + // Deliberately not a loop: how much room a page bought is a fact about the + // *layout* it produced, and no layout has happened yet inside this coroutine + // -- so a loop would be re-reading the height from before the page it just + // fetched and would ask for the whole conversation. Letting the measurement + // answer means each page is checked against what it actually added, and a + // page that folds into almost no new rows is followed by another because the + // room genuinely did not grow. + loadOlderPage() } catch (_: ApiException) { // Leave `moreHistory` alone: the next scroll asks again. } finally { @@ -1454,90 +1375,62 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () ) } - // Laid out from the bottom, with the newest message at index 0. + // Every loaded row composed and kept, hanging from the newest message. // - // The obvious arrangement -- oldest first, then scroll to the end - // -- opens at the top and travels the whole transcript to get - // where it belongs. On an imported session that is nine hundred - // items measured before anything is readable, seen as the view - // visibly racing downward every time it opened. + // The obvious arrangement -- oldest first, then scroll to the end -- opens at the top and + // travels the whole transcript to get where it belongs. On an imported session that was + // nine hundred rows measured before anything was readable, seen as the view visibly racing + // downward every time it opened. Scrolling the content in reverse removes the journey + // rather than hiding it: position zero *is* the newest message, so the first frame is + // already the right one and nothing has to be scrolled at all. See [TranscriptScroll]. // - // Anchoring at the bottom removes the journey rather than hiding - // it: the first frame is already the newest message, and older - // ones are composed only as somebody scrolls back to them, which - // is also what makes history cheap on a long conversation. - // Empty until a saved position has been put back -- see the opening effect. Held out of - // the list rather than drawn and scrolled, so there is no frame in which the transcript is - // somewhere other than where it was left. - val drawnRows = if (restoring) emptyList() else rows.asReversed() + // Drawn only once there is nothing left to put back, and measured throughout -- the + // heights are what a saved position is expressed in, so the rows have to be laid out + // before it can be applied. Held out of the drawing rather than out of the list, so there + // is no frame in which the transcript is somewhere other than where it was left. + val settled = !restoring && !listState.settling Box(Modifier.weight(1f).fillMaxWidth()) { - LazyColumn( - state = listState, - reverseLayout = true, - modifier = Modifier.fillMaxSize(), - contentPadding = PaddingValues(16.dp), - // Bottom, and it has to be said: `reverseLayout` defaults the arrangement to - // `Bottom` on its own, but naming `spacedBy` replaces that default with - // `spacedBy`'s own, which is `Top`. The arrangement is what places the content - // when there is less of it than the viewport -- so a session whose loaded rows - // did not fill the screen drew them against the *top*, leaving a gap between the - // newest message and the box you type in, and no room to scroll the gap away. - // Opening the keyboard shrank the viewport enough for the content to overflow it - // and the list snapped down, which is what made it look like a scrolling fault - // rather than a placement one. - verticalArrangement = Arrangement.spacedBy(8.dp, Alignment.Bottom), - ) { - // The last thing in the transcript, because that is where - // they are in the session's reading of events: after - // everything it has taken in, and not yet taken in - // themselves. What the session is *doing* about them is a - // line below, in [SessionStatusRow]. - if (!restoring && itemsAboveRows() > 0) { - item(key = "queued") { - Column(horizontalAlignment = Alignment.End) { - waitingCommands.forEach { (_, text) -> - CommandBubble(text, waiting = true) - } - queued.forEach { waiting -> - UserBubble( - settings = settings, - sessionId = summary.id, - text = waiting.text, - images = waiting.images, - pending = true, - ) + BoxWithConstraints(Modifier.fillMaxSize()) { + TranscriptColumn( + rows = rows, + state = listState, + // A scrollable child is measured with no height bound, so the content cannot + // ask how tall the visible area is; this is the only place that knows. + viewportHeight = maxHeight, + contentPadding = TRANSCRIPT_PADDING, + spacing = TRANSCRIPT_SPACING, + modifier = + Modifier.fillMaxSize().drawWithContent { if (settled) drawContent() }, + below = { + // The last thing in the transcript, because that is where they are in the + // session's reading of events: after everything it has taken in, and not + // yet taken in themselves. What the session is *doing* about them is a + // line below, in [SessionStatusRow]. + if (queued.isNotEmpty() || waitingCommands.isNotEmpty()) { + Column(horizontalAlignment = Alignment.End) { + waitingCommands.forEach { (_, text) -> + CommandBubble(text, waiting = true) + } + queued.forEach { waiting -> + UserBubble( + settings = settings, + sessionId = summary.id, + text = waiting.text, + images = waiting.images, + pending = true, + ) + } } } - } - } - // Reversed to match the layout, so index 0 is the newest and - // the reader still sees them in the order they happened. - // Grouped first: adjacent tool calls collapse into one row, - // which is a decision about this screen and not about the - // transcript the stream and paging share. - // Keyed, and this is what stops the list moving under whoever is reading - // it. Every new message is an insertion at index 0 here, so without a key the - // rows keep their positions and the content slides through them -- which looks - // exactly like the view scrolling by itself. The keys above matter for the same - // reason: the working indicator appearing and disappearing is another insertion - // at the same end. Paging older history is the opposite insertion and was - // already fine, and stays fine, because a key survives both. - items(drawnRows, key = { it.key }) { row -> + }, + ) { row -> val bounds = remember { RowBounds() } Box( Modifier.onGloballyPositioned { bounds.top = it.positionInRoot().y bounds.height = it.size.height.toFloat() } - .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, - ) - } + .holdTopEdge(row.key, topEdgeHeld) { grew -> listState.by(grew) } ) { when (row) { is TranscriptRow.Tools -> @@ -1584,8 +1477,37 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () text = item.text, images = item.images, ) - is TranscriptItem.AssistantMsg -> - AssistantMessage(item.text, replies) + is TranscriptItem.AssistantMsg -> { + // The newest reply is never cut. It is the one being read + // as it arrives -- often still arriving -- and putting a + // "show the rest" under a turn somebody is waiting for + // hides the answer they are waiting for. Every reply + // behind it is history, and history is what this is for. + val shortened = + if ( + item.seq == newestReply || + item.seq in expandedReplies + ) + null + else remember(item.text) { shortenedReply(item.text) } + if (shortened == null) { + AssistantMessage(item.text, replies) + } else { + CappedReply( + full = item.text, + shortened = shortened, + replies = replies, + onShowMore = { + // Anchored like every other row that changes + // height, so the edge the reader touched + // stays where it is. + toggleAnchored(row.key, bounds, bounds.top) { + expandedReplies = expandedReplies + item.seq + } + }, + ) + } + } is TranscriptItem.ToolRun -> ToolCard( tool = item, @@ -1668,7 +1590,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // // In the middle of the transcript rather than at either end, because it is not // reporting on the newest message or the oldest; it is standing in for all of them. - if (!ready || restoring) { + // `settled` and not `restoring` alone, so the spinner covers the whole wait: fetching + // the history a saved position needs, and then the frames between those rows arriving + // and the layout that measures them putting the position back. They are the two halves + // of the same wait and the transcript is not drawn for either. + if (!ready || !settled) { CircularProgressIndicator(Modifier.align(Alignment.Center).size(LOADING_SPINNER)) } @@ -1686,22 +1612,14 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () Surface( // Instantly. An animated scroll travels the whole transcript, so the // further back somebody has read the longer this takes -- the one press - // whose cost grows with how much there is to skip, which is backwards. The - // list is keyed and composes only what it lands on, so going straight there - // costs the same from anywhere. - onClick = { - // Says what it means as well as doing it. Pressing this is the reader - // choosing the newest end, which is exactly what [followTail] records -- - // and nothing else here would notice, because a snap moves the list - // inside one frame and the listener that decides [followTail] waits for a - // scroll to *end*. So this used to land at the bottom with following - // still switched off, and the next message did not bring the view with - // it: the reader pressed "take me to the latest" and then sat watching a - // conversation that had moved on without them. Sibling to the anchor - // above, which had the same hole and lost the saved position instead. - followTail = true - scope.launch { listState.scrollToItem(0) } - }, + // whose cost grows with how much there is to skip, which is backwards. + // + // Arriving there is all this has to do now. The newest end is where the + // content hangs from, so being at it is the whole of following it, and there + // is no separate flag to set -- which is what this press used to forget, + // landing the reader at the bottom with new messages not bringing the view + // with them. + onClick = { scope.launch { listState.scroll.scrollTo(0) } }, shape = CircleShape, color = MaterialTheme.colorScheme.surfaceContainerHigh, modifier = diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt new file mode 100644 index 0000000..15b8515 --- /dev/null +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptScroll.kt @@ -0,0 +1,204 @@ +package com.example.aiapp + +import androidx.compose.foundation.ScrollState +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.heightIn +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll +import androidx.compose.runtime.Composable +import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.layout.onPlaced +import androidx.compose.ui.layout.positionInParent +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp + +/** + * The transcript's scroll position, addressed the way the transcript itself is: a row, and how far + * that row has been scrolled past the top of the viewport. + * + * Pixels are the unit underneath because this list is a plain [Column] rather than a lazy one -- + * every loaded row is composed, measured and kept, so there is a real height for every row whether + * or not it is on screen, and no index has to stand in for one. What that buys is that scrolling + * back over a message never rebuilds it: a Compose node that is still alive and whose constraints + * have not changed is not re-measured at all (`MeasurePassDelegate.remeasure` returns early), so + * the text keeps the layout it was given. A lazy list disposes a row the moment it leaves the + * viewport, and the markdown tree, the measured lines and the cached paragraph go with it -- which + * is the lag when scrolling back over history that has already been read. + * + * The content is laid out oldest-first and scrolled in reverse, which is what makes the two + * insertions this list actually gets free rather than corrected: + * + * - `reverseScrolling` places the content by its *end*, so `value == 0` is the newest message and + * growing the content at the far end -- a page of older history landing -- moves nothing on + * screen. `maxValue` grows and `value` is left alone. + * - A new message extends the same end the viewport is pinned to, so following it is not an effect + * that has to notice and correct: at `value == 0` the newest content is simply what is on screen. + * The keyboard opening is the same case from the other side -- the viewport shrinks, `maxValue` + * grows, and the newest message is still against the bottom. + * + * Both of those were scroll corrections in the lazy version, each with a comment explaining a way + * it had been seen to fire at the wrong moment. + */ +@Stable +class TranscriptScroll(internal val scroll: ScrollState) { + + /** + * Where each row's top edge sits inside the content, by the seq that names it. + * + * Written from the layout pass as rows are placed, so it describes the layout that is on + * screen. Keyed on [TranscriptRow.startSeq] rather than on the row's display key for the reason + * the anchor is: a tool run is renamed when the newest page starts somewhere new, and a + * position recorded against the old name is never found again. + */ + private val tops = HashMap() + + /** + * A position waiting to be put back, applied by the layout that first places its row. + * + * Held here rather than applied by whoever loaded the row because the pixels do not exist yet + * at that point: a plain column has no height for a row until it has been measured. Applying it + * from the placement is what makes the restore frame-exact -- the transcript is drawn where it + * was left rather than drawn at the newest end and then moved, which is a journey the reader + * would see. [settling] is the same fact asked the other way, and gates drawing. + */ + var pending: ScrollAnchor? by mutableStateOf(null) + private set + + /** Whether a saved position is still being put back, so nothing should be drawn yet. */ + val settling: Boolean + get() = pending != null + + internal fun placed(seq: Long, top: Int) { + tops[seq] = top + pending?.let { anchor -> + if (anchor.seq != seq) return@let + scroll.dispatchRawDelta( + (scroll.maxValue - top - anchor.offset - scroll.value).toFloat() + ) + pending = null + } + } + + /** Everything these described is gone -- a stream reset, or a different session. */ + fun clear() = tops.clear() + + /** Whether the newest message is on screen. See the class comment: the newest end is zero. */ + val atNewest: Boolean + get() = scroll.value == 0 + + /** + * How much unread history sits above the viewport, in pixels. + * + * This is the paging question asked in the unit it is actually about. Counting rows could not + * answer it: a row is anything from one line to a screenful, so "three rows back" is a + * different amount of reading depending on which three, and the count that mattered -- how far + * somebody can scroll before running out -- was never what was measured. + */ + val roomAbove: Int + get() = scroll.maxValue - scroll.value + + /** The height of the visible area, 0 until the first measurement. */ + val viewport: Int + get() = scroll.viewportSize + + /** Where the reader is now, or null before anything has been laid out. */ + fun anchor(): ScrollAnchor? { + val top = scroll.maxValue - scroll.value + // The row covering the top of the viewport: the last one that starts at or above it. + val at = tops.entries.filter { it.value <= top }.maxByOrNull { it.value } ?: return null + return ScrollAnchor(at.key, top - at.value) + } + + /** + * Asks for [anchor] to be put back by the layout that places its row; see [pending]. + * + * The caller is responsible for the row being loaded. Nothing here can wait for one that never + * arrives, and a position held open for it would leave the transcript blank -- so a row that is + * not in the transcript any more is [giveUp]'s case, not this one. + */ + fun restore(anchor: ScrollAnchor) { + pending = anchor + } + + /** Draw where we are instead: the anchored row is not in this transcript any more. */ + fun giveUp() { + pending = null + } + + /** + * Scrolls by [pixels] without animating, from wherever the caller is. + * + * Used to hold a row's top edge still when it changes height. Growth already goes upward + * because the content hangs from its end, so the bottom edge is held for free and this is the + * other case; see `holdTopEdge`. + */ + fun by(pixels: Int) { + scroll.dispatchRawDelta(pixels.toFloat()) + } +} + +@Composable +fun rememberTranscriptScroll(key: Any?): TranscriptScroll { + val scroll = rememberScrollState() + return remember(key) { TranscriptScroll(scroll) } +} + +/** + * Every loaded row, composed and kept. See [TranscriptScroll] for why this is not a lazy list. + * + * [viewportHeight] is passed in rather than measured here because a scrollable child is measured + * with no height bound at all, so the content cannot ask how tall the visible area is. It is what + * holds a conversation shorter than the screen against the bottom, where the composer is, instead + * of leaving a gap under it that cannot be scrolled away. + */ +@Composable +fun TranscriptColumn( + rows: List, + state: TranscriptScroll, + viewportHeight: Dp, + contentPadding: PaddingValues, + spacing: Dp, + modifier: Modifier = Modifier, + below: @Composable () -> Unit, + row: @Composable (TranscriptRow) -> Unit, +) { + Column( + modifier + .verticalScroll(state.scroll, reverseScrolling = true) + .padding(contentPadding) + .heightIn(min = viewportHeight) + .fillMaxWidth(), + verticalArrangement = Arrangement.spacedBy(spacing, Alignment.Bottom), + ) { + rows.forEach { item -> + // Keyed so that a row keeps its composition -- and so the state inside it, an open + // tool call or an expanded reply, stays with the row rather than with the position. + key(item.key) { + Column( + Modifier.fillMaxWidth().onPlaced { + state.placed(item.startSeq, it.positionInParent().y.toInt()) + } + ) { + row(item) + } + } + } + below() + } +} + +/** The gap between rows, and the room around the whole conversation. */ +val TRANSCRIPT_SPACING: Dp = 8.dp + +val TRANSCRIPT_PADDING: PaddingValues = PaddingValues(16.dp)