diff --git a/AGENTS.md b/AGENTS.md index 7c5782e..cd241c2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -217,6 +217,27 @@ first if a remote spawn ever mangles an argument. server binds wg0, which exists here but is unreachable from the emulator (it dials 10.0.2.2). First run prints the enrollment QR/URI with the token — capture it from the log. +- **`app/debug-transcript.sh` puts a real conversation on the emulator.** + The echo driver stays the right rig for most things and is the wrong one + for anything whose cost scales with what was actually written: a real + reply is longer, is real markdown, and carries tool calls whose input and + output are kilobytes rather than a word. Two faults were invisible until + a real transcript was loaded — a page of history landing mid-fling threw + the reader back to the newest end, and parsing one real reply took 51ms + against 4.6ms for a synthetic one. `-b` takes the biggest conversation on + the machine rather than the newest, which is what a scrolling test wants; + `--stop` takes it all down again. + It copies the transcript into `/tmp` and gives the server a `HOME` of its + own, so the import can only see the copy — importing spawns `claude + --resume`, and against the real file that is a second CLI writing to a + conversation somebody may still be in. **A transcript never goes in this + repository**: they hold whatever was said, read and written in that + session, and `~/repos` is shared with the host besides. +- **`ai-server --delay MS` holds every response back.** Over the tunnel a + phone's requests take tens to hundreds of milliseconds, and several + faults live entirely in what the app does *while* one is outstanding. On + a loopback server those windows close before anything can be observed, + so the bug looks like it is not there. - Prefer exercising the server directly over going through the UI: `curl --cacert ~/.config/ai-app/certs/ca.pem -H "Authorization: Bearer …" https://127.0.0.1:8443/sessions`. The CA is wherever `--certs` put it — by default under diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt index eeb4efa..3ed092e 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Markdown.kt @@ -15,6 +15,7 @@ import com.mikepenz.markdown.m3.markdownColor import com.mikepenz.markdown.m3.markdownTypography import com.mikepenz.markdown.model.State import com.mikepenz.markdown.model.parseMarkdown +import java.util.concurrent.ConcurrentHashMap import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -29,9 +30,9 @@ import kotlinx.coroutines.withContext * are the same Catppuccin values the rest of the app uses. Nothing here picks a colour of its own. */ @Composable -fun MarkdownText(text: String, modifier: Modifier = Modifier) { +fun MarkdownText(text: String, replies: ParsedReplies, modifier: Modifier = Modifier) { val body = MaterialTheme.typography.bodyLarge - val parsed = parsedMarkdown(text) + val parsed = parsedMarkdown(text, replies) Markdown( parsed, colors = @@ -125,12 +126,49 @@ fun MarkdownText(text: String, modifier: Modifier = Modifier) { * real prefix of the reply rather than a guess at it; it is simply one parse behind. */ @Composable -private fun parsedMarkdown(text: String): State { +private fun parsedMarkdown(text: String, replies: ParsedReplies): State { // The text each parse came from, so the first composition's is not immediately repeated. - val parsed = remember { mutableStateOf(text to parseMarkdown(text)) } + val parsed = remember { mutableStateOf(text to replies.of(text)) } LaunchedEffect(text) { if (parsed.value.first == text) return@LaunchedEffect + // Not through [replies]: this is a reply still arriving, and every delta would leave + // another copy of a message that is about to be superseded. parsed.value = text to withContext(Dispatchers.Default) { parseMarkdown(text) } } return parsed.value.second } + +/** + * Replies parsed before the row that draws them is composed. + * + * Parsing is the expensive half of drawing a reply, and it is expensive in proportion to how much + * was written. Measured against a real Claude Code transcript on the emulator, one message took + * **51ms** and several took 10-25ms, against 4.6ms for the short synthetic replies this was first + * tuned on -- so a page of history landing composed several rows that each stalled the frame they + * appeared in. That is the lag when a block loads. + * + * Nothing here changes what a row does when it has no answer waiting: it parses inline, on the + * composing thread, because a row measured at nothing before it is measured at its real height + * collapses the transcript above it. The point is only that by the time the reader scrolls to a + * row, the answer is usually already made -- [warm] runs on a background thread as each page of + * history arrives, which is seconds before anybody reaches the rows it brought. + * + * A miss is not stored, and that is what bounds this: the map holds one entry per message a page + * warmed and nothing else, so a reply still streaming cannot fill it with hundreds of copies of + * itself on the way to being finished. It is dropped with the screen, and emptied by the stream + * reset that drops the rows it describes. + */ +class ParsedReplies { + private val parsed = ConcurrentHashMap() + + /** The parse of [text] -- the one made ahead, or one made now. */ + fun of(text: String): State = parsed[text] ?: parseMarkdown(text) + + /** Parses whatever is not held yet. Call off the composing thread; that is the whole point. */ + fun warm(texts: List) { + texts.forEach { text -> parsed.computeIfAbsent(text) { parseMarkdown(it) } } + } + + /** Everything these described is gone; see [ParsedReplies]. */ + fun clear() = parsed.clear() +} 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 e298c6e..70ac150 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/MemoryNote.kt @@ -26,24 +26,42 @@ import androidx.compose.ui.unit.dp * seconds away, and a half-written marker is not a marker yet. */ @Composable -fun AssistantMessage(text: String, modifier: Modifier = Modifier) { - val parts = remember(text) { splitMemoryNotes(text) } - if (parts.size == 1 && parts[0] is MessagePart.Prose) { - MarkdownText(text, modifier) +fun AssistantMessage(text: String, replies: ParsedReplies, modifier: Modifier = Modifier) { + val parts = remember(text) { partsOf(text) } + val only = parts.singleOrNull() + if (only is MessagePart.Prose) { + MarkdownText(only.text, replies, modifier) return } Column(modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(6.dp)) { parts.forEach { part -> when (part) { - is MessagePart.Prose -> MarkdownText(part.text) - is MessagePart.Remembered -> MemoryNote(part) + is MessagePart.Prose -> MarkdownText(part.text, replies) + is MessagePart.Remembered -> MemoryNote(part, replies) } } } } +/** + * The pieces [AssistantMessage] draws, which is [splitMemoryNotes] with one correction. + * + * A reply carrying no notes is drawn from the message as it arrived rather than from the trimmed + * prose part made while looking for them -- inspecting a message must not change it. That belongs + * here rather than at the two places that need the answer, because [markdownIn] has to name the + * same strings this draws: a string warmed under a key no row ever looks up is a miss that nothing + * reports, and the row pays the parse in the frame it appears, which is the cost being removed. + */ +private fun partsOf(text: String): List { + val parts = splitMemoryNotes(text) + 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 } + @Composable -private fun MemoryNote(note: MessagePart.Remembered) { +private fun MemoryNote(note: MessagePart.Remembered, replies: ParsedReplies) { Card(Modifier.fillMaxWidth()) { Column(Modifier.padding(12.dp)) { // Named, not just tinted: a colour can say "this one is different", but it cannot say @@ -54,16 +72,19 @@ private fun MemoryNote(note: MessagePart.Remembered) { style = MaterialTheme.typography.labelSmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) - MarkdownText(note.text, Modifier.padding(top = 4.dp)) + MarkdownText(note.text, replies, Modifier.padding(top = 4.dp)) } } } /** One piece of a reply: ordinary prose, or a sentence attributed to a memory file. */ sealed class MessagePart { - data class Prose(val text: String) : MessagePart() + /** The markdown this piece is drawn from. */ + abstract val text: String - data class Remembered(val text: String, val files: List) : MessagePart() + data class Prose(override val text: String) : MessagePart() + + data class Remembered(override val text: String, val files: List) : MessagePart() } private val MEMORY_NOTE = diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt index e2b3ef3..c540c08 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt @@ -32,6 +32,7 @@ fun PeerMessageRow( item: TranscriptItem.PeerNote, expanded: Boolean, onToggle: (Float) -> Unit, + replies: ParsedReplies, modifier: Modifier = Modifier, ) { Card(modifier.fillMaxWidth().clickableAt(onToggle)) { @@ -50,7 +51,7 @@ fun PeerMessageRow( ) } } - if (expanded) MarkdownText(item.text, Modifier.padding(top = 6.dp)) + if (expanded) MarkdownText(item.text, replies, Modifier.padding(top = 6.dp)) } } } 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 f7bcd3e..3fe9ec6 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -533,6 +533,19 @@ private fun updateTool( if (it is TranscriptItem.ToolRun && it.id == id) change(it) else it } +/** + * Parses the replies among [rows], off whatever thread is drawing. + * + * Called where a page of transcript is folded rather than where a row is composed, which is the + * whole point: the work happens seconds before the reader reaches the rows it was done for. See + * [ParsedReplies]. + */ +private suspend fun warm(replies: ParsedReplies, rows: List) { + val texts = rows.filterIsInstance().flatMap { markdownIn(it.text) } + if (texts.isEmpty()) return + withContext(Dispatchers.Default) { replies.warm(texts) } +} + @Composable fun SessionScreen( settings: ServerSettings, @@ -613,6 +626,9 @@ fun SessionScreen( var moreHistory by remember { mutableStateOf(true) } var loadingHistory by remember { mutableStateOf(false) } var ready by remember { mutableStateOf(false) } + // 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 @@ -775,6 +791,7 @@ fun SessionScreen( try { val page = withContext(Dispatchers.IO) { fetchTranscript(settings, summary.id) } page.forEach { apply(it) } + warm(replies, items) } catch (e: ApiException) { // Not fatal: the stream below still replays from zero, which is // slow but complete. Saying so beats silently showing nothing. @@ -816,6 +833,7 @@ fun SessionScreen( // screen -- `apply` refills them, and scrolling // up pages the rest back in as it always does. items = listOf() + replies.clear() held = listOf() oldestSeq = 0L moreHistory = true @@ -899,11 +917,28 @@ fun SessionScreen( // 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. + // + // 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 && count > 0) listState.scrollToItem(0) } + .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. @@ -959,7 +994,14 @@ fun SessionScreen( earlier = foldEvent(earlier, entry) } } - items = joinPages(earlier, items) + val joined = joinPages(earlier, items) + // After the join rather than on the page alone: a boundary that fell + // through a reply leaves `joinPages` holding a message made of both + // halves, and that text has existed for no time at all. Warming the page + // by itself warmed the two halves and missed the one thing drawn -- + // which showed up as a single 22ms parse surviving every page. + warm(replies, joined) + items = joined have = groupToolRuns(items).size } } catch (_: ApiException) { @@ -1260,7 +1302,8 @@ fun SessionScreen( text = item.text, images = item.images, ) - is TranscriptItem.AssistantMsg -> AssistantMessage(item.text) + is TranscriptItem.AssistantMsg -> + AssistantMessage(item.text, replies) is TranscriptItem.ToolRun -> ToolCard( tool = item, @@ -1319,6 +1362,7 @@ fun SessionScreen( PeerMessageRow( item = item, expanded = item.seq in expandedNotes, + replies = replies, onToggle = { at -> toggleAnchored(row.key, bounds, at) { expandedNotes = diff --git a/app/debug-transcript.sh b/app/debug-transcript.sh new file mode 100755 index 0000000..40f86c9 --- /dev/null +++ b/app/debug-transcript.sh @@ -0,0 +1,149 @@ +#!/bin/sh +# Puts a real Claude Code conversation on the emulator, for looking at the +# transcript screen under content it was not written against. +# +# The echo driver's fixtures (`/mixed`, `/stream`) are the right rig for most +# things and the wrong one for anything whose cost scales with what was +# actually written: a real reply is longer, is real markdown, and carries tool +# calls whose input and output are kilobytes rather than a word. Two faults +# were invisible until a real transcript was loaded -- a page of history +# landing mid-fling threw the reader back to the newest end, and parsing one +# real reply took 51ms against 4.6ms for a synthetic one. +# +# ./debug-transcript.sh # newest transcript in ~/.claude/projects +# ./debug-transcript.sh dev-updater # newest one whose project path matches +# ./debug-transcript.sh -b dev-updater # the biggest one instead of the newest +# ./debug-transcript.sh -d 350 # hold every response back 350ms +# +# **The transcript never enters the repository.** These files are private -- +# they hold whatever was said, read and written in that session -- so this +# copies one into /tmp and points an isolated server at it. Nothing it makes +# is committed, and ~/repos is shared with the host besides. +# +# What it builds, all of it disposable: +# /tmp/ai-app-debug/home a HOME holding only the copied transcript, so +# the import cannot see or resume a live session +# /tmp/ai-app-debug/sessions that server's own data directory +# a server on PORT, with its own config and the real CA (so the installed +# APK, which pins the CA of the machine that built it, still trusts it) +set -eu + +PORT="${PORT:-8455}" +DELAY=0 +MATCH="" +BIGGEST="" +STOP="" +while [ $# -gt 0 ]; do + case "$1" in + -d|--delay) DELAY="$2"; shift 2 ;; + -b|--biggest) BIGGEST=yes; shift ;; + --stop) STOP=yes; shift ;; + -h|--help) sed -n '2,29p' "$0" | sed 's/^# \{0,1\}//'; exit 0 ;; + *) MATCH="$1"; shift ;; + esac +done + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +cd "$SCRIPT_DIR" +REPO=$(dirname "$SCRIPT_DIR") +WORK=/tmp/ai-app-debug +PROJECTS="$HOME/.claude/projects" + +# Whatever the last run left, before this one takes the port again. +# +# Importing spawns `claude --resume` so the conversation can be continued, and +# those outlive the server that started them: twelve accumulated over one +# afternoon of re-running this. They are found by the scratch HOME and nothing +# else, because every other `claude` on this machine is somebody's live session +# -- including the one that may be running this script. +stop_previous() { + for pid in $(pgrep -x claude 2>/dev/null); do + home=$(tr '\0' '\n' <"/proc/$pid/environ" 2>/dev/null | sed -n 's/^HOME=//p') + if [ "$home" = "$WORK/home" ]; then kill "$pid" 2>/dev/null || true; fi + done + pkill -f "[a]i-server --bind 127.0.0.1 --port $PORT" 2>/dev/null || true + # Gone, not merely signalled: the next start binds the same port. + while pgrep -f "[a]i-server --bind 127.0.0.1 --port $PORT" >/dev/null 2>&1; do sleep 1; done +} + +stop_previous +if [ -n "$STOP" ]; then + echo "Stopped the debug server on port $PORT and anything it spawned." + exit 0 +fi + +# Newest first, so with no argument you get the conversation you were just in. +# `--biggest` is the other question worth asking of this directory, and the one +# a scrolling test wants: the longest conversation on the machine is the one +# with enough rows to page backwards through, and the newest is routinely a +# session five minutes old with nothing in it. +if [ -n "$BIGGEST" ]; then + SRC=$(ls -S "$PROJECTS"/*"$MATCH"*/*.jsonl 2>/dev/null | head -1) +else + SRC=$(ls -t "$PROJECTS"/*"$MATCH"*/*.jsonl 2>/dev/null | head -1) +fi +if [ -z "$SRC" ]; then + echo "No Claude Code transcript under $PROJECTS matching '${MATCH:-anything}'." >&2 + echo "Sessions are written there as /.jsonl." >&2 + exit 1 +fi +ID=$(basename "$SRC" .jsonl) +PROJECT=$(basename "$(dirname "$SRC")") +echo "==> Using $PROJECT/$ID ($(wc -l < "$SRC") lines, $(du -h "$SRC" | cut -f1))" + +# A HOME of its own is the isolation: `import::list` enumerates +# "$HOME"/.claude/projects/*/*.jsonl through the transport, so a server started +# with this one can only ever see the copy. That matters for more than tidiness +# -- importing spawns `claude --resume `, and against the real file that +# would be a second CLI writing to a conversation somebody may still be in. +rm -rf "$WORK" +mkdir -p "$WORK/home/.claude/projects/$PROJECT" +cp "$SRC" "$WORK/home/.claude/projects/$PROJECT/$ID.jsonl" + +CERTS="${XDG_CONFIG_HOME:-$HOME/.config}/ai-app/certs" +if [ ! -f "$CERTS/ca.pem" ]; then + echo "No CA at $CERTS/ca.pem -- start ai-server once normally first." >&2 + exit 1 +fi + +SERVER="$REPO/server/target/debug/ai-server" +[ -x "$SERVER" ] || (cd "$REPO/server" && cargo build) + +echo "==> Starting server on port $PORT (delay ${DELAY}ms)" +HOME="$WORK/home" setsid nohup "$SERVER" \ + --bind 127.0.0.1 --port "$PORT" \ + --config "$WORK/config.ron" --data-dir "$WORK/sessions" --certs "$CERTS" \ + --delay "$DELAY" >"$WORK/server.log" 2>&1 /dev/null; do sleep 1; done +TOKEN=$(grep -o 'token=[A-Za-z0-9_-]*' "$WORK/server.log" | head -1 | cut -d= -f2) + +api() { curl -s --cacert "$CERTS/ca.pem" -H "Authorization: Bearer $TOKEN" "$@"; } + +echo "==> Importing" +SETUP=$(api "https://127.0.0.1:$PORT/setups" | sed -n 's/.*"id":"\([^"]*\)".*/\1/p' | head -1) +SESSION=$(api -H 'Content-Type: application/json' -X POST \ + "https://127.0.0.1:$PORT/sessions" \ + -d "{\"setup\":\"$SETUP\",\"provider\":\"claude-cli\",\"title\":\"$PROJECT\",\"import\":\"$ID\"}" \ + | sed -n 's/.*"id":"\([^"]*\)".*/\1/p' | head -1) +echo " session $SESSION, $(wc -l < "$WORK/sessions/$SESSION/transcript.jsonl") events" + +# 10.0.2.2 is the emulator's route to this VM's loopback. The `&` are quoted +# on the *device* side: adb runs its argument through a shell there, which +# would otherwise cut the URI at the first one and enrol with no token. +if command -v adb >/dev/null 2>&1 && [ -n "$(adb devices | awk '$2=="device"{print $1}')" ]; then + echo "==> Enrolling the app" + adb shell "am start -a android.intent.action.VIEW \ + -d 'aiapp://enroll?host=10.0.2.2&port=$PORT&token=$TOKEN'" >/dev/null +fi + +cat < Result<()> { auth::require_token, )); + // Outside the auth layer, so an unauthenticated request is refused at + // the speed it always was: this is here to slow the app down, not to + // widen the window on anything guessing at tokens. + let app = match args.delay { + 0 => app, + ms => { + tracing::warn!("delaying every response by {ms}ms -- development override"); + app.layer(axum::middleware::from_fn( + move |request, next: Next| async move { + tokio::time::sleep(Duration::from_millis(ms)).await; + next.run(request).await + }, + )) + } + }; + let addr = SocketAddr::new(bind_ip, args.port); tracing::info!("serving https://{addr}");