diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ScrollAnchor.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ScrollAnchor.kt index dd2f15a..8d63f1b 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ScrollAnchor.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ScrollAnchor.kt @@ -8,16 +8,19 @@ private const val ANCHORS = "session-scroll" /** * Where a session's transcript was left, so reopening it lands where reading stopped. * - * Named by the *row* rather than by an index, because an index means nothing across a reopen: the - * transcript is fetched newest-first and a session that has said anything since has renumbered - * every position. The row's own key survives all of it -- it is the same value the list is keyed - * by, which is what already stops the view moving when history pages in. + * Named by a **sequence number** -- see [TranscriptRow.startSeq] -- rather than by an index or by + * the row key the list draws with. An index means nothing across a reopen, since the transcript is + * fetched newest-first and a session that has said anything since has renumbered every position. + * The row key looks stable and is not: a tool row is named after its run, `joinPages` gives a run + * the name of its newest half, and the newest half is whatever the newest page happened to start + * with -- so an active session renames its tool runs every time it is reopened, and an anchor + * naming one is never found. A seq is the server's own numbering, assigned once and never moved. * * [offset] is how far into that row the viewport starts, in pixels, and is the reason this is a - * pair rather than a key: a reader stopped halfway down a long tool output is put back halfway down - * it. + * pair rather than a bare seq: a reader stopped halfway down a long tool output is put back halfway + * down it. */ -data class ScrollAnchor(val key: String, val offset: Int) +data class ScrollAnchor(val seq: Long, val offset: Int) /** * On this device rather than on the backend, which is where this app otherwise keeps state so every @@ -29,10 +32,9 @@ fun loadScrollAnchor(context: Context, sessionId: String): ScrollAnchor? { val stored = context.getSharedPreferences(ANCHORS, Context.MODE_PRIVATE).getString(sessionId, null) ?: return null - // Offset first so the split is unambiguous: a row key is an arbitrary string and may contain - // anything, where the offset is digits. - val offset = stored.substringBefore(':').toIntOrNull() ?: return null - return ScrollAnchor(stored.substringAfter(':'), offset) + val seq = stored.substringBefore(':').toLongOrNull() ?: return null + val offset = stored.substringAfter(':').toIntOrNull() ?: return null + return ScrollAnchor(seq, offset) } /** @@ -46,6 +48,6 @@ fun loadScrollAnchor(context: Context, sessionId: String): ScrollAnchor? { fun saveScrollAnchor(context: Context, sessionId: String, anchor: ScrollAnchor?) { context.getSharedPreferences(ANCHORS, Context.MODE_PRIVATE).edit { if (anchor == null) remove(sessionId) - else putString(sessionId, "${anchor.offset}:${anchor.key}") + else putString(sessionId, "${anchor.seq}:${anchor.offset}") } } 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 2a8445d..9cd1909 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -113,14 +113,6 @@ private const val HISTORY_SCREENS = 3 */ private const val HISTORY_PAGE = 800 -/** - * The list key of the bubble holding what has been sent and not read yet. - * - * Named rather than written at the `item` that draws it, because a saved scroll position stores - * whatever key it was left on and this is one of the values that can be. - */ -private const val QUEUED_KEY = "queued" - /** * Which row was asked to hold its top edge, and how tall it was when it last measured. * @@ -844,19 +836,37 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () */ fun itemsAboveRows() = if (queued.isNotEmpty() || waitingCommands.isNotEmpty()) 1 else 0 - /** - * Where the row named [key] sits in the list, or null when it is not loaded. - * - * Computed from `items` rather than from `rows` for the reason [loadOlderPage] gives. - */ /** Everything the list draws, in items rather than in rows. See [itemsAboveRows]. */ fun listItemCount() = groupToolRuns(items).size + itemsAboveRows() - fun indexOfKey(key: String): Int? { - // The bubble is not a row, and it is above all of them. - if (key == QUEUED_KEY) return if (itemsAboveRows() > 0) 0 else null - val row = groupToolRuns(items).asReversed().indexOfFirst { it.key.toString() == key } - return if (row < 0) null else row + itemsAboveRows() + /** + * The row drawn at list index [index], or null when that item is not a row. + * + * 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. + * + * Computed from `items` rather than from `rows`, as [rowAt] is and for the same reason. + */ + 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. + val ordered = groupToolRuns(items) + val row = ordered.indexOfLast { it.startSeq <= seq } + if (row < 0) return null + return ordered.size - 1 - row + itemsAboveRows() } /** @@ -975,15 +985,22 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // the one worth restoring -- somebody who read to the bottom has no anchor at all -- // and the cost was already paid on the way down there. savedAnchor?.let { anchor -> - // Pages until the row is loaded and has something older behind it. The oldest - // loaded row is a half-row: `joinPages` welds the other half onto it when the - // page behind it arrives, and it grows -- so anchoring into one puts the reader - // where they were only until the next page lands. Any row that is not the oldest - // is final. Landing a screen and a half out was what this cost. - var index = indexOfKey(anchor.key) + // Pages until the anchor's row is loaded and has something older behind it. The + // oldest loaded row is a half-row: `joinPages` welds the other half onto it when + // the page behind it arrives, and it grows -- so anchoring into one puts the + // reader where they were only until the next page lands, which landed a screen + // and a half out. Any row that is not the oldest is final. + // + // This terminates because `oldestSeq` walks strictly backwards and the anchor is + // a seq: once the window reaches past it, some loaded row starts at or before it + // and [indexOfSeq] answers. Keying on the row's *name* instead could not promise + // 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)) { if (!loadOlderPage()) break - index = indexOfKey(anchor.key) + 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 @@ -1099,34 +1116,52 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () } } // 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. Written - // where a scroll settles, below. + // 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 at the newest end, and so as an instruction to forget where the reader - // was. That wiped every saved position on the way in, before the restore below could - // use it. Only the transitions after it are scrolls. + // 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) return@collect - followTail = atNewest - // Written where the answer settles, for the same reason [followTail] is: mid-fling - // is not where anybody left off. Cleared at the newest end rather than recorded, - // because that is where a session with nothing to restore opens anyway -- so the - // ordinary case costs a `remove` and no page-back on the way in. + .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 + // *programmatic* scroll moves the list within one frame, so `isScrollInProgress` never + // observably changes and anything waiting for a settle never runs. Jump to latest is exactly + // 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. + 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. + .drop(1) + .collect { settled -> + if (settled == null) return@collect saveScrollAnchor( context, summary.id, - if (atNewest) null - else - listState.layoutInfo.visibleItemsInfo.firstOrNull()?.let { first -> - ScrollAnchor( - first.key.toString(), - listState.firstVisibleItemScrollOffset, - ) - }, + // 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) }, ) } } @@ -1426,7 +1461,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // themselves. What the session is *doing* about them is a // line below, in [SessionStatusRow]. if (!restoring && itemsAboveRows() > 0) { - item(key = QUEUED_KEY) { + item(key = "queued") { Column(horizontalAlignment = Alignment.End) { waitingCommands.forEach { (_, text) -> CommandBubble(text, waiting = true) @@ -1622,7 +1657,19 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // 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 = { scope.launch { listState.scrollToItem(0) } }, + 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) } + }, shape = CircleShape, color = MaterialTheme.colorScheme.surfaceContainerHigh, modifier = diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt index 807e221..af2b8b9 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt @@ -61,9 +61,25 @@ sealed class TranscriptRow { */ abstract val key: Any + /** + * Where this row starts in the transcript: the sequence number of the oldest event behind it. + * + * Separate from [key], and deliberately so. [key] is the list's identity and is a display + * decision -- a tool row is named after its run, and a run takes its name from whichever call + * was first when it was folded, which changes as pages arrive. A seq is the server's own + * numbering: it is assigned once, never moves, and means the same thing to every device. So + * anything that has to point at a place in the conversation and still find it later -- a saved + * scroll position is the one -- points with this, and anything that has to identify a row + * within one composition uses [key]. + */ + abstract val startSeq: Long + data class Single(val item: TranscriptItem) : TranscriptRow() { override val key: Any get() = (item as? TranscriptItem.ToolRun)?.runId ?: item.seq + + override val startSeq: Long + get() = item.seq } /** Two or more calls with nothing between them; drawn as one collapsed card. */ @@ -74,6 +90,9 @@ sealed class TranscriptRow { override val key: Any get() = id + + override val startSeq: Long + get() = calls.first().seq } }