diff --git a/AGENTS.md b/AGENTS.md index 20c5c03..81375b5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -408,16 +408,19 @@ belongs in `~/.claude/TOOLCHAIN.md` or `~/.claude/MACHINE.md` instead. the reader hit the end of what was loaded on every swipe and stood there for a round trip. It is `HISTORY_SCREENS` viewports now, counted from what is actually on screen. -- **A page landing while the history observer was fetching it could spend the - layout change meant to ask for the next one.** The observer collected only - `LazyListState.layoutInfo`; while its collector was suspended in - `loadOlderPage`, a compact page could be composed and laid out without - leaving another change to observe afterward. Codex exposed it because a - page full of calls collapses into one tool group: loading stopped until - expanding that group forced a layout. The observer is also keyed on - `oldestSeq` now, so every successful page restarts it against the settled - layout. A failed page does not advance that cursor and still waits for the - next scroll instead of retrying in a loop. +- **A page landing while the history observer was fetching it must trigger its + own successor.** The observer once collected only `LazyListState.layoutInfo`; + while its collector was suspended in `loadOlderPage`, a compact page could + be composed and laid out without leaving another change to observe afterward. + Keying the effect on `oldestSeq` still missed the opening prefetch: that key + changed while `loadingHistory` was true, so the restarted effect declined to + overlap it and never noticed the flag returning to false. Codex exposes both + failures because a page full of calls collapses into one tool group: loading + stopped until expanding that group forced a layout. The observer now collects + the cursor, loading, restoring and failure state with the layout, so returning + to not-loading always rechecks the settled height. A failed page turns the + history boundary into a Try again control rather than retrying in a loop or + requiring another scroll. - **Only `fetchTranscript` was off the main thread; the fold was not.** `foldEvent` returns a new list per event, so a page is that many copies of a growing list — fine at 80 events and about 300,000 element copies at 800, 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 13bfc91..fc3e9f1 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width +import androidx.compose.foundation.lazy.LazyListLayoutInfo import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.text.selection.rememberSelectionState @@ -156,6 +157,16 @@ private const val RESTORE_PAGE_MAX = 4000 */ private const val RESTORE_PAGE_CUSHION = 400 +/** Every input that can make the history boundary need another request. */ +private data class HistoryLoadSignal( + val layout: LazyListLayoutInfo, + val restoring: Boolean, + val moreHistory: Boolean, + val loading: Boolean, + val error: String?, + val oldestSeq: Long, +) + /** * Which row was asked to hold its top edge, and how tall it was when it last measured. * @@ -369,6 +380,7 @@ fun SessionScreen( val running = sessionWorking(status) var moreHistory by remember { mutableStateOf(true) } var loadingHistory by remember { mutableStateOf(false) } + var historyError by remember(address, epoch) { mutableStateOf(null) } var ready by remember { mutableStateOf(false) } // Replies parsed ahead of the rows that draw them; see [ParsedReplies]. val replies = remember(address) { ParsedReplies() } @@ -429,6 +441,7 @@ fun SessionScreen( held = listOf() oldestSeq = 0L moreHistory = true + historyError = null // A send the server could not accept is this phone's only copy. A reset replaces server // state, not that local outbox, so dropping it here would eat the message a second time. replaceQueued(queued.filter { it.local }) @@ -659,6 +672,21 @@ fun SessionScreen( return true } + /** One guarded history request, shared by automatic paging and the explicit retry control. */ + suspend fun requestOlderPage(): Boolean { + if (loadingHistory) return false + loadingHistory = true + historyError = null + return try { + loadOlderPage() + } catch (e: ApiException) { + historyError = e.message ?: "Unknown error" + false + } finally { + loadingHistory = false + } + } + // A call opened on its own stays open when a second call in the same run turns it into a group. // Until this, watching a Bash call and having the session make another one shut the one being // read and folded it behind "Called 2 tools". @@ -844,14 +872,7 @@ fun SessionScreen( // scroll met it and waited a round trip. So the first full page goes right behind it, while // the screen is already up. A restore skips this: it has just paged as deep as it needed. if (savedAnchor == null && moreHistory && !loadingHistory) { - loadingHistory = true - try { - loadOlderPage() - } catch (_: ApiException) { - // The next scroll asks again. - } finally { - loadingHistory = false - } + requestOlderPage() } // Last, and off this thread: this session is what must not be evicted, so it is marked as // visited before the budget is applied, and both are a walk of the cache directory. @@ -1041,18 +1062,38 @@ fun SessionScreen( // There is no correction beside this one. Following the newest message is not an effect: the // list is reversed, so an arriving message extends the end the viewport is pinned to. val unitSizes = remember(address) { HashMap() } - // A successful page advances `oldestSeq` even when its collapsed rows add too little height to - // produce another layout after this collector returns. Restarting on that cursor makes the - // promised re-check happen; a failed page leaves it unchanged and still waits for a scroll. - LaunchedEffect(listState, moreHistory, oldestSeq) { - snapshotFlow { listState.layoutInfo } - .collect { info -> + // Every state that can make another request useful is part of the collected value. In + // particular, the opening prefetch used to advance `oldestSeq` while `loadingHistory` was still + // true; the effect restarted, declined to overlap it, and never noticed when loading became + // false. A tool expansion happened to cause the next layout and unstick it. Collecting the + // loading transition itself makes a compact page immediately ask for its successor. + LaunchedEffect(listState) { + snapshotFlow { + HistoryLoadSignal( + listState.layoutInfo, + restoring, + moreHistory, + loadingHistory, + historyError, + oldestSeq, + ) + } + .collect { signal -> + val info = signal.layout val visible = info.visibleItemsInfo if (visible.isEmpty()) return@collect // Before the guards below, so sizes keep accumulating while a page is in flight and // the next estimate starts better informed. visible.forEach { unitSizes[it.key] = it.size } - if (restoring || !moreHistory || loadingHistory) return@collect + if ( + signal.restoring || + !signal.moreHistory || + signal.loading || + signal.error != null || + signal.oldestSeq == 0L + ) { + return@collect + } val viewport = info.viewportSize.height if (viewport == 0) return@collect val loaded = currentUnits @@ -1066,16 +1107,11 @@ fun SessionScreen( room += unitSizes[loaded[index].key] ?: average if (room >= cushion) return@collect } - loadingHistory = true - try { - // One page, and then this fires again if it was not enough -- the estimate is - // re-made from what the page actually added. - loadOlderPage() - } catch (_: ApiException) { - // Leave `moreHistory` alone: the next scroll asks again. - } finally { - loadingHistory = false - } + // One page, and then this fires again if it was not enough -- the estimate is + // re-made when loading returns to false, even when folding the page did not change + // the list's height at all. A failure leaves `moreHistory` alone but records an + // error, stopping this loop until the boundary's Try again control is pressed. + requestOlderPage() } } @@ -1476,6 +1512,11 @@ fun SessionScreen( units = units, state = listState, moreHistory = moreHistory, + historyError = historyError, + // A button means "make the request", not merely "let the estimator decide + // again". Its measurements can be stale after the tall error row is + // replaced by the spinner, and that used to make a press do nothing. + onRetryHistory = { scope.launch { requestOlderPage() } }, selection = selection, modifier = Modifier.fillMaxSize().drawWithContent { if (settled) drawContent() }, diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt index 6e271fa..5da4a9c 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt @@ -1,6 +1,7 @@ package com.example.aiapp import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding @@ -10,6 +11,9 @@ import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.foundation.text.selection.SelectionState import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -49,6 +53,8 @@ fun TranscriptList( units: List, state: LazyListState, moreHistory: Boolean, + historyError: String?, + onRetryHistory: () -> Unit, selection: SelectionState, modifier: Modifier = Modifier, below: @Composable () -> Unit, @@ -94,15 +100,29 @@ fun TranscriptList( DebugStats.count("unit composed") Box(Modifier.fillMaxWidth().padding(top = u.gap)) { unit(u) } } - // Standing in for everything not fetched yet. Only here while there is more -- its - // appearance at the top edge is also roughly when the next page is asked for, so what - // it reports is a fetch in flight rather than an end reached. + // Standing in for everything not fetched yet. A failed fetch stays actionable here: + // when the loaded transcript is too short to scroll, this boundary is the only place + // the reader can be given another way to ask. if (moreHistory) { item(key = "history", contentType = "history") { Box(Modifier.fillMaxWidth().padding(vertical = 24.dp)) { - CircularProgressIndicator( - Modifier.align(Alignment.Center).size(HISTORY_SPINNER) - ) + if (historyError == null) { + CircularProgressIndicator( + Modifier.align(Alignment.Center).size(HISTORY_SPINNER) + ) + } else { + Column( + Modifier.align(Alignment.Center), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + "Couldn't load earlier messages. $historyError", + color = MaterialTheme.colorScheme.error, + style = MaterialTheme.typography.bodySmall, + ) + TextButton(onClick = onRetryHistory) { Text("Try again") } + } + } } } }