Draw an opened peer message as list units, and give a peer note its own key
A peer message opened was still one item of the transcript list, so every
block of it was composed, measured, placed and kept *alive* while any part
of it was on screen -- and the framework's own per-frame cost grows with how
many nodes are alive rather than how many are drawn. Measured on the
emulator, scrolling the same stretch with a 43KB message shut, opened, and
opened after this change:
shut opened opened, split
draw phase per frame 0.81ms 3.85ms 1.22ms
of that, the framework 0.39ms 3.15ms 0.42ms
frame total, median 16.9ms 24.3ms 21.1ms
So an opened message now costs about what a shut one does. That is the
shape the report from the phone had -- 11.80ms of draw phase with 79% of it
outside anything this app times -- which no counter here could attribute,
because "2 units visible" says one of them is enormous without saying which.
It says which now: the transcript section of the report names every visible
unit and its height, which is what found this.
The card is cut up rather than given up. A filled Material card is elevation
zero (`FilledCardTokens.ContainerElevation` is `Level0`), so there is no
shadow for a seam to show through: each piece paints the same fill, rounds
only the corners at the ends of the message, and keeps the 12dp inset the
card's own column had. Opening and shutting still hold the edge the reader
pressed, and now without a correction -- the list is keyed, so it holds the
item it is anchored on wherever the new ones land.
The crash this turned up is the more serious half. `placePeerNote` gives a
note the seq of the turn it started so it sorts above the reply it caused,
and argued the seq was free because it belongs to a status change and a
status draws no row. True, and about the wrong collision: two messages that
arrive during one turn are stamped with the same turn, so they became two
rows with one key and `LazyColumn` threw -- the app dying in the middle of
somebody reading. Two agents writing to a session mid-turn is an ordinary
afternoon. A note now keeps its own arrival seq as its identity while `seq`
stays the position it sorts at, and which value a row is keyed by moved onto
`TranscriptItem` itself, which also removes the `as? ToolRun` branch that
was doing the same job in `TranscriptRow.Single`.
`transcriptUnits` now says which two units collided if it ever happens
again. All the framework's message carries is the key, and when that key is
a seq it names neither row; two lines here answered in one run what had
taken an afternoon.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
80aaf286c2
commit
917eb9a7b3
6 files changed
+321
-51
No files matched your search
@@ -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<TranscriptRow>, replies: ParsedReplies): List<TranscriptUnit> {
|
||||
fun transcriptUnits(
|
||||
rows: List<TranscriptRow>,
|
||||
replies: ParsedReplies,
|
||||
openNotes: Set<Long>,
|
||||
): List<TranscriptUnit> {
|
||||
val units = ArrayList<TranscriptUnit>(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<TranscriptRow>, replies: ParsedReplies): List<Tra
|
||||
}
|
||||
}
|
||||
units.reverse()
|
||||
reportDuplicateKeys(units)
|
||||
return units
|
||||
}
|
||||
|
||||
/**
|
||||
* Says which two units share a key, before the list dies of it.
|
||||
*
|
||||
* A duplicate key is fatal -- `LazyColumn` throws, and the app goes down in the middle of somebody
|
||||
* reading a conversation -- and all the framework's message carries is the key. When that key is a
|
||||
* seq it names neither row, and there is no way back from it to how the two came to share one: it
|
||||
* took an afternoon and a fixture that could reproduce it. Two lines here answered it immediately,
|
||||
* naming both rows and the field they had in common ([TranscriptItem.PeerNote.arrived]).
|
||||
*
|
||||
* Always on, for the same reason [DebugStats] is: an instrument that is only in the build nobody is
|
||||
* holding when it breaks is not an instrument. It costs one map over the units that were just
|
||||
* built, beside a loop that already allocates one entry per unit.
|
||||
*/
|
||||
private fun reportDuplicateKeys(units: List<TranscriptUnit>) {
|
||||
val seen = HashMap<Any, TranscriptUnit>()
|
||||
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<TranscriptUnit>, visible: List<LazyListItemInfo>, 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.
|
||||
*
|
||||
|
||||
Reference in new issue
Block a user