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 fe62c15..b7d0567 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/PeerMessage.kt @@ -1,5 +1,6 @@ package com.example.aiapp +import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -7,12 +8,14 @@ import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width -import androidx.compose.material3.Card +import androidx.compose.foundation.shape.CornerSize +import androidx.compose.material3.CardDefaults import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp @@ -28,35 +31,92 @@ import androidx.compose.ui.unit.dp * transcript that puts it in their voice is making a claim about who asked for the work that * follows -- which is exactly the question a peer message is usually the answer to. * - * Opened, it is drawn a block at a time ([BlockedMarkdown]) for the reason every reply already is: - * these are the longest messages a transcript holds, and one of them as a single render is one - * parse and one display list proportional to the whole of it. See [markdownBlocks]. + * Opened, the card is drawn in *pieces* -- this heading and one [PeerBlockRow] per markdown block, + * each its own item of the transcript list. See [TranscriptUnit.PeerHead] for the measurements that + * bought; what matters here is that the pieces have to add up to the card that was there before, so + * the fill, the corner radius and the padding all live in [peerSurface] rather than being written + * out at each piece. */ @Composable -fun PeerMessageRow( +fun PeerHeadRow( item: TranscriptItem.PeerNote, - expanded: Boolean, + open: Boolean, onToggle: () -> Unit, - replies: ParsedReplies, modifier: Modifier = Modifier, ) { - Card(modifier.fillMaxWidth().clickable(onClick = onToggle)) { - Column(Modifier.padding(12.dp)) { - Row(verticalAlignment = Alignment.CenterVertically) { - Text("Message from ${item.from}", style = MaterialTheme.typography.titleSmall) - if (!expanded) { - Spacer(Modifier.width(8.dp)) - Text( - item.text.lineSequence().firstOrNull { it.isNotBlank() }.orEmpty(), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - maxLines = 1, - // The head, not the tail: a message is identified by how it opens. - overflow = TextOverflow.Ellipsis, - ) - } + Column(modifier.peerSurface(top = true, bottom = !open, onToggle = onToggle)) { + Row(verticalAlignment = Alignment.CenterVertically) { + Text("Message from ${item.from}", style = MaterialTheme.typography.titleSmall) + if (!open) { + Spacer(Modifier.width(8.dp)) + Text( + item.text.lineSequence().firstOrNull { it.isNotBlank() }.orEmpty(), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + maxLines = 1, + // The head, not the tail: a message is identified by how it opens. + overflow = TextOverflow.Ellipsis, + ) } - if (expanded) BlockedMarkdown(item.text, replies, Modifier.padding(top = 6.dp)) } } } + +/** + * One block of an opened peer message, on the same card the heading started. + * + * Clickable like the heading, so the card still shuts wherever it is pressed -- it was one control + * before it was several items, and which piece the finger lands on is not something the reader + * chose. + */ +@Composable +fun PeerBlockRow( + text: String, + replies: ParsedReplies, + last: Boolean, + onToggle: () -> Unit, + modifier: Modifier = Modifier, +) { + Column(modifier.peerSurface(top = false, bottom = last, onToggle = onToggle)) { + // The gap the card's own column used to provide between its heading and its prose, and + // between one block and the next. Uniform, because both of those were 6dp already. + MarkdownText(text, replies, Modifier.padding(top = BLOCK_SPACING)) + } +} + +/** + * One piece of a peer message's card: the fill, the corners it owns, and the room inside it. + * + * A filled Material card is elevation zero ([CardDefaults] takes it from `FilledCardTokens`, which + * is `Level0`), so there is no shadow that a seam would show through -- which is the whole reason + * the card can be cut up at all. Each piece paints the same container colour a + * [androidx.compose .material3.Card] would and rounds only the corners at the ends of the message, + * so the pieces abut into one continuous card. + * + * The padding is the other half of it: 12dp all round was the card's own, so the top piece keeps + * the top of it, the bottom piece the bottom, and the middle pieces neither. + */ +@Composable +private fun Modifier.peerSurface(top: Boolean, bottom: Boolean, onToggle: () -> Unit): Modifier { + val square = CornerSize(0.dp) + val shape = + MaterialTheme.shapes.medium.copy( + topStart = if (top) MaterialTheme.shapes.medium.topStart else square, + topEnd = if (top) MaterialTheme.shapes.medium.topEnd else square, + bottomStart = if (bottom) MaterialTheme.shapes.medium.bottomStart else square, + bottomEnd = if (bottom) MaterialTheme.shapes.medium.bottomEnd else square, + ) + return fillMaxWidth() + .clip(shape) + .background(CardDefaults.cardColors().containerColor) + .clickable(onClick = onToggle) + .padding( + start = PEER_PADDING, + end = PEER_PADDING, + top = if (top) PEER_PADDING else 0.dp, + bottom = if (bottom) PEER_PADDING else 0.dp, + ) +} + +/** The room inside a peer message's card, which was `Card { Column(padding(12.dp)) }`. */ +private val PEER_PADDING = 12.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 685f57f..f1f9fce 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -343,7 +343,7 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () // What is actually drawn: the transcript with runs of adjacent tool // calls folded into one row each, flattened into the list's units. val rows = remember(items) { groupToolRuns(items) } - val units = remember(rows) { transcriptUnits(rows, replies) } + val units = remember(rows, expandedNotes) { transcriptUnits(rows, replies, expandedNotes) } // The same list, readable from effects launched before this composition: an effect's closure // keeps the values of the composition that launched it, and both the anchor saver and the // restore need the units as they are *now*. @@ -967,6 +967,22 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () openMemories = if (text in openMemories) openMemories - text else openMemories + text } + /** + * Opens or closes one peer message, from whichever of its pieces was pressed. + * + * By seq rather than by unit, because an open message is several units and all of them shut it + * -- it was one card before it was several items, and which piece the finger landed on is not + * something the reader chose. + * + * No [toggleAnchored] here, and that is the difference between growing a row and adding items: + * the list is keyed, so it holds the item it is anchored on wherever the new ones land. What + * the reader tapped keeps its place because the list keeps it, not because a measurement + * corrected it afterwards. + */ + fun togglePeer(seq: Long) { + expandedNotes = if (seq in expandedNotes) expandedNotes - seq else expandedNotes + seq + } + fun act(onDone: () -> Unit = {}, action: () -> Unit) { scope.launch { try { @@ -1147,6 +1163,11 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () " ${listState.layoutInfo.viewportSize.height}px," + " ${listState.layoutInfo.visibleItemsInfo.size}" + " units visible", + visibleUnits( + units, + listState.layoutInfo.visibleItemsInfo, + UNITS_START, + ), " ${expandedTools.size} tool calls and" + " ${expandedGroups.size} groups open", ), @@ -1283,6 +1304,19 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () ) { unit -> when (unit) { is TranscriptUnit.Block -> MarkdownText(unit.text, replies) + is TranscriptUnit.PeerHead -> + PeerHeadRow( + unit.item, + unit.open, + onToggle = { togglePeer(unit.item.seq) }, + ) + is TranscriptUnit.PeerBlock -> + PeerBlockRow( + unit.text, + replies, + unit.last, + onToggle = { togglePeer(unit.seq) }, + ) is TranscriptUnit.Memory -> MemoryNote( unit.part, @@ -1481,20 +1515,13 @@ fun SessionScreen(settings: ServerSettings, summary: SessionSummary, onBack: () is TranscriptItem.ClearedNote -> ClearedRow() is TranscriptItem.CompactedNote -> CompactedRow(item) + // Never reached: a peer message is flattened + // into its own units, so it is not a whole row. + // Here because a `when` over the item kinds has to + // stay exhaustive, and drawing nothing is how a + // row that stopped being handled would look. is TranscriptItem.PeerNote -> - PeerMessageRow( - item = item, - expanded = item.seq in expandedNotes, - replies = replies, - onToggle = { - toggleAnchored(row) { - expandedNotes = - if (item.seq in expandedNotes) - expandedNotes - item.seq - else expandedNotes + item.seq - } - }, - ) + PeerHeadRow(item, open = false, onToggle = {}) } } } 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 8a93503..11fb2b4 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/ToolRows.kt @@ -64,7 +64,8 @@ sealed class TranscriptRow { * and it is the *same* value whether the run is drawn as one card or as a group. A lone call * that gains a neighbour becomes a group without changing identity, which is the case a * seq-based key got wrong: the row the reader was looking at was replaced rather than updated. - * Everything else keys on the seq of the event behind it, which never moves. + * Which value that is belongs to the item ([TranscriptItem.key]), not to a `when` here: a row + * is one item and the item is what knows what it is called. */ abstract val key: Any @@ -83,7 +84,7 @@ sealed class TranscriptRow { data class Single(val item: TranscriptItem) : TranscriptRow() { override val key: Any - get() = (item as? TranscriptItem.ToolRun)?.runId ?: item.seq + get() = item.key override val startSeq: Long get() = item.seq diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt index f650b7b..1428b02 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptItems.kt @@ -26,6 +26,18 @@ sealed class TranscriptItem { */ abstract val seq: Long + /** + * This item's identity on screen, which is its [seq] for everything that has one of its own. + * + * Here rather than in [TranscriptRow.Single] because the two items that need something else are + * the two that know why: a tool call is named after its run, and a peer note is *sorted* by the + * turn it started rather than by where it arrived. Asking each item what it is called is also + * what stops the next such item being missed -- a `when` over concrete types in the row would + * have to gain a case, silently, and nothing says when it did not. + */ + open val key: Any + get() = seq + data class UserMsg( override val seq: Long, val text: String, @@ -72,7 +84,11 @@ sealed class TranscriptItem { * breaks -- a screenshot loaded on one page and its call on the next read as unrelated. */ val images: List = emptyList(), - ) : TranscriptItem() + ) : TranscriptItem() { + /** A run, not a seq: see [TranscriptRow.key] for what that identity has to survive. */ + override val key: Any + get() = runId + } data class QuestionCard( override val seq: Long, @@ -97,8 +113,24 @@ sealed class TranscriptItem { * * Its own row rather than a [UserMsg]: see [PeerMessageRow] for why the voice matters. */ - data class PeerNote(override val seq: Long, val from: String, val text: String) : - TranscriptItem() + data class PeerNote( + override val seq: Long, + val from: String, + val text: String, + /** + * The seq of the event this note came in on, which is what makes it itself. + * + * [seq] is where the note *sorts*, and [placePeerNote] sets it to the seq the turn began at + * so the note is drawn above the reply it caused. Two messages that arrive during one turn + * therefore share a seq -- and sharing an identity as well killed the app, because the + * transcript list refuses two items with one key. Two agents writing to a session mid-turn + * is an ordinary afternoon, not a corner. + */ + val arrived: Long = seq, + ) : TranscriptItem() { + override val key: Any + get() = arrived + } /** * A command the session ran on itself -- `/compact`, `/rename`. @@ -278,8 +310,10 @@ private fun adoptRun( * where it belongs rather than being drawn out of order at the end. * * Taking the turn's opening seq as its own is also what keeps the list sorted, which anchors and - * paging both depend on. That seq belongs to a status change, and a status draws no row, so there - * is nothing for it to collide with. + * paging both depend on. It is only a *position*, though, and the note keeps its own arrival seq as + * its identity ([TranscriptItem.PeerNote.arrived]). The argument for sharing was that the turn's + * seq belongs to a status change and a status draws no row -- true, and it answered the wrong + * question: what two notes stamped with the same turn collide with is each other. * * Without a stamp -- a message replayed out of a session file, which is already in the right place * -- it stays where it arrived. @@ -290,7 +324,7 @@ private fun placePeerNote( event: SessionEvent.PeerMessage, ): List { val at = event.turnStart ?: return items + TranscriptItem.PeerNote(seq, event.from, event.text) - val note = TranscriptItem.PeerNote(at, event.from, event.text) + val note = TranscriptItem.PeerNote(at, event.from, event.text, arrived = seq) val index = items.indexOfFirst { it.seq > at } if (index < 0) return items + note val behind = (items.getOrNull(index - 1) as? TranscriptItem.ToolRun)?.runId 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 9fabfe3..8618784 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt @@ -88,6 +88,7 @@ fun TranscriptList( }, ) { // The bottom of the screen: what is waiting to be read sits under the newest message. + // The one item before the units, which is what [UNITS_START] counts. item(key = "below", contentType = "below") { below() } items(count = units.size, key = { units[it].key }, contentType = { units[it]::class }) { val u = units[it] @@ -111,6 +112,15 @@ fun TranscriptList( } } +/** + * How many of the list's own items come before the first unit -- the waiting-messages slot. + * + * Named once because two things count on it: the list building itself, and [visibleUnits] turning a + * list index back into the unit that was drawn there. Read off by hand at the second of those, it + * is an off-by-one that misnames every row in a report and looks like a plausible answer. + */ +const val UNITS_START = 1 + /** The gap between rows, and the room around the whole conversation. */ val TRANSCRIPT_SPACING: Dp = 8.dp diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptUnits.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptUnits.kt index 396b15d..6ccac94 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptUnits.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptUnits.kt @@ -1,5 +1,6 @@ package com.example.aiapp +import androidx.compose.foundation.lazy.LazyListItemInfo import androidx.compose.runtime.Immutable import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp @@ -59,6 +60,53 @@ sealed class TranscriptUnit { get() = "b$seq:$ordinal" } + /** + * The heading of a message from another agent: who sent it, and the control that opens it. + * + * A peer message is the one row whose *opened* size is unbounded -- these are the longest + * things a transcript holds -- so it is flattened the same way a settled reply is, and for the + * same reason: as one item, every block of it is composed, measured, placed and kept alive + * while any part of it is on screen. Measured on the emulator, opening a 43KB one took the + * transcript's share of the draw phase from 0.81ms a frame to 3.85ms, and the framework's own + * per-frame bookkeeping -- which grows with how many nodes are *alive* -- from 0.39ms to + * 3.15ms. + * + * The card is drawn in pieces rather than given up: a filled Material card is elevation zero, + * so it has no shadow to break, and each piece paints the same fill with only the corners it + * owns. See [PeerHeadRow] and [PeerBlockRow]. + */ + data class PeerHead( + override val seq: Long, + val item: TranscriptItem.PeerNote, + val open: Boolean, + override val gap: Dp, + ) : TranscriptUnit() { + /** + * The note's own key, so opening and shutting does not change what the list is anchored on + * -- and so two notes stamped with one turn's seq are still two items. See + * [TranscriptItem.PeerNote]. + */ + override val key: Any + get() = item.key + + override val ordinal: Int + get() = 0 + } + + /** One markdown block of an opened peer message; [last] is the piece that closes the card. */ + data class PeerBlock( + override val seq: Long, + override val ordinal: Int, + val text: String, + val last: Boolean, + override val gap: Dp, + /** The note this block belongs to; its key, not its seq. See [TranscriptItem.PeerNote]. */ + val note: Any, + ) : TranscriptUnit() { + override val key: Any + get() = "p$note:$ordinal" + } + /** One memory note of a settled reply; see [MemoryNote]. */ data class Memory( override val seq: Long, @@ -76,21 +124,46 @@ sealed class TranscriptUnit { * screen, which is what a reversed lazy list calls the start. * * Every settled reply is cut into its blocks ([markdownBlocks], via the caches on [replies] so a - * message is only ever split once). The reply still arriving -- the last row -- stays whole: its - * text changes with every delta, and splitting it here would parse the whole message per delta on - * whichever thread is composing. [AssistantMessage]'s own streaming path already parses deltas off - * the main thread and gives the live message a layer per block. + * message is only ever split once), and so is an *opened* peer message -- [openNotes] is which ones + * those are, which is why the flatten needs it. A shut one is a single heading and cannot be worth + * splitting. The reply still arriving -- the last row -- stays whole: its text changes with every + * delta, and splitting it here would parse the whole message per delta on whichever thread is + * composing. [AssistantMessage]'s own streaming path already parses deltas off the main thread and + * gives the live message a layer per block. * * Runs per fold, so it must stay proportional to what is loaded with no parsing in it on the warm * path: [ParsedReplies.partsOf] and [ParsedReplies.blocksOf] are lookups for any text [warm] has * seen, and a miss -- the one message that just finished streaming -- costs its split exactly once. */ -fun transcriptUnits(rows: List, replies: ParsedReplies): List { +fun transcriptUnits( + rows: List, + replies: ParsedReplies, + openNotes: Set, +): List { val units = ArrayList(rows.size) rows.forEachIndexed { index, row -> val rowGap = if (index == 0) 0.dp else TRANSCRIPT_SPACING val item = (row as? TranscriptRow.Single)?.item - if (item is TranscriptItem.AssistantMsg && index != rows.lastIndex) { + if (item is TranscriptItem.PeerNote) { + val open = item.seq in openNotes + units += TranscriptUnit.PeerHead(row.startSeq, item, open, rowGap) + // No gap between the pieces: they are one card, and a card with a stripe through it is + // what any spacing here would draw. + if (open) { + val blocks = replies.blocksOf(item.text) + blocks.forEachIndexed { at, block -> + units += + TranscriptUnit.PeerBlock( + row.startSeq, + at + 1, + block, + last = at == blocks.lastIndex, + gap = 0.dp, + note = item.key, + ) + } + } + } else if (item is TranscriptItem.AssistantMsg && index != rows.lastIndex) { var ordinal = 0 fun gap() = if (ordinal == 0) rowGap else BLOCK_SPACING replies.partsOf(item.text).forEach { part -> @@ -111,9 +184,74 @@ fun transcriptUnits(rows: List, replies: ParsedReplies): List) { + val seen = HashMap() + units.forEach { unit -> + val had = seen.put(unit.key, unit) + if (had != null) { + android.util.Log.w("ai-app", "duplicate unit key ${unit.key}: $had AND $unit") + } + } +} + +/** + * What is on screen right now, a unit at a time: what each one is and how tall it is. + * + * For the render report, and it is the line every "it is slow here" report has needed. The + * framework's own per-frame cost grows with how many nodes are *alive* rather than how many are on + * screen, so a screen holding one enormous item is slow in a way that no counter of ours + * distinguishes from a screen holding twenty ordinary ones -- and "2 units visible" says one of + * them is enormous without saying which. This says which. + * + * [first] is the index the list gave the first *unit*: the list also holds the waiting-messages + * slot at index zero and the history spinner past the end, and both are named here rather than + * silently reported as whichever unit is nearest. + */ +fun visibleUnits(units: List, visible: List, first: Int): String = + if (visible.isEmpty()) " nothing on screen" + else + " on screen: " + + visible.joinToString(", ") { info -> + "${units.getOrNull(info.index - first).kind} ${info.size}px" + } + +/** What a unit is, in a word, for [visibleUnits]. Null is one of the list's own non-unit items. */ +private val TranscriptUnit?.kind: String + get() = + when (this) { + null -> "the list's own" + is TranscriptUnit.Block -> "reply block" + is TranscriptUnit.PeerHead -> if (open) "peer heading (open)" else "peer heading" + is TranscriptUnit.PeerBlock -> "peer block" + is TranscriptUnit.Memory -> "memory note" + is TranscriptUnit.Whole -> + when (val row = row) { + is TranscriptRow.Tools -> "tool group" + // The class name rather than a word per kind: this is a diagnostic, and a + // `when` here would be one more place that has to gain a case whenever the + // transcript does -- silently naming a new row after an old one until somebody + // noticed. + is TranscriptRow.Single -> row.item::class.simpleName.orEmpty() + } + } + /** * Where the unit named by a saved position sits in [units], or null if its row is not loaded. *