Name a run of tool calls once, instead of after whichever call is first
A group of adjacent tool calls was identified by its first call, and the list is keyed by that identity. But a run can gain members at *either* end -- a new call arriving beside it, or a page of history arriving in front of it -- so its first member is not a name, it is a description that changes. Every time it changed, the row was a different row as far as the list was concerned: the anchor went with it, and the transcript stepped under whoever was reading. Each call now carries the run it belongs to, decided once when it is folded in and never recomputed, and the row keys on that. A lone call that gains a neighbour becomes a group *without* changing identity, which the old key got wrong in the other direction too -- one row was replaced by another rather than updated. `joinPages` hands the arriving older calls the name of the run they are joining, rather than renaming that run after them. The obvious way round is the wrong one: the newer half is the part already on screen, so naming the joined run after the older half renames the row the reader is looking at, which is the whole failure this is meant to remove. Checked against the same twelve-`/tools 8` rig, whose page boundary falls inside the second group: every group still reads eight, so the grouping is unchanged -- what changed is that none of their identities move. Toward the standing rule for this screen, which is that it may only move when the reader is at the newest end and something new arrives.
This commit is contained in:
1 parent
2264723ee3
commit
50670863f4
2 files changed
+73
-15
No files matched your search
@@ -93,6 +93,17 @@ sealed class TranscriptItem {
|
|||||||
data class ToolRun(
|
data class ToolRun(
|
||||||
override val seq: Long,
|
override val seq: Long,
|
||||||
val id: String,
|
val id: String,
|
||||||
|
/**
|
||||||
|
* The run of adjacent calls this one belongs to, named once when the call is folded in and
|
||||||
|
* never recomputed.
|
||||||
|
*
|
||||||
|
* Carried rather than derived because a run can gain members at *either* end -- a new call
|
||||||
|
* arriving beside it, or a page of history arriving in front of it -- so no function of its
|
||||||
|
* current members is stable. It is the first call's id at the moment the run started, which
|
||||||
|
* is a name rather than a description: [joinPages] hands it to older calls that turn out to
|
||||||
|
* belong to the same run, instead of renaming the run they joined.
|
||||||
|
*/
|
||||||
|
val runId: String,
|
||||||
val tool: String,
|
val tool: String,
|
||||||
val input: String,
|
val input: String,
|
||||||
val output: String,
|
val output: String,
|
||||||
@@ -177,6 +188,16 @@ sealed class TranscriptItem {
|
|||||||
) : TranscriptItem()
|
) : TranscriptItem()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The run a call joins: the one it lands next to, or a new one named after itself.
|
||||||
|
*
|
||||||
|
* Only ever consulted when the call is first folded in. That is what makes the name stable -- a run
|
||||||
|
* keeps whatever it was called when it started, however many calls arrive at either end of it
|
||||||
|
* afterwards.
|
||||||
|
*/
|
||||||
|
private fun runIdFor(items: List<TranscriptItem>, id: String): String =
|
||||||
|
(items.lastOrNull() as? TranscriptItem.ToolRun)?.runId ?: id
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Puts a page of older items in front of the ones already loaded, healing any tool call the page
|
* Puts a page of older items in front of the ones already loaded, healing any tool call the page
|
||||||
* boundary cut in two.
|
* boundary cut in two.
|
||||||
@@ -220,7 +241,28 @@ fun joinPages(earlier: List<TranscriptItem>, later: List<TranscriptItem>): List<
|
|||||||
row
|
row
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return healed + later.filterNot { it is TranscriptItem.ToolRun && it.id in endedLater }
|
val kept = later.filterNot { it is TranscriptItem.ToolRun && it.id in endedLater }
|
||||||
|
return adoptRun(healed, kept) + kept
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Hands the older calls at the join the name of the run they are joining.
|
||||||
|
*
|
||||||
|
* The two pages were folded separately, so a run split by the boundary came back as two runs with
|
||||||
|
* two names. Naming the joined run after the *older* half would be the obvious way round and is the
|
||||||
|
* wrong one: the newer half is the part already on screen, and renaming it is renaming the row the
|
||||||
|
* reader is looking at, which is how a list loses its anchor and steps under them. So the arriving
|
||||||
|
* calls take the name of the ones already there, and nothing visible changes identity.
|
||||||
|
*/
|
||||||
|
private fun adoptRun(
|
||||||
|
earlier: List<TranscriptItem>,
|
||||||
|
later: List<TranscriptItem>,
|
||||||
|
): List<TranscriptItem> {
|
||||||
|
val joining = (later.firstOrNull() as? TranscriptItem.ToolRun)?.runId ?: return earlier
|
||||||
|
val tail = earlier.takeLastWhile { it is TranscriptItem.ToolRun }
|
||||||
|
if (tail.isEmpty()) return earlier
|
||||||
|
return earlier.dropLast(tail.size) +
|
||||||
|
tail.map { (it as TranscriptItem.ToolRun).copy(runId = joining) }
|
||||||
}
|
}
|
||||||
|
|
||||||
fun foldEvent(items: List<TranscriptItem>, entry: SeqEvent): List<TranscriptItem> =
|
fun foldEvent(items: List<TranscriptItem>, entry: SeqEvent): List<TranscriptItem> =
|
||||||
@@ -242,6 +284,7 @@ fun foldEvent(items: List<TranscriptItem>, entry: SeqEvent): List<TranscriptItem
|
|||||||
TranscriptItem.ToolRun(
|
TranscriptItem.ToolRun(
|
||||||
entry.seq,
|
entry.seq,
|
||||||
event.id,
|
event.id,
|
||||||
|
runIdFor(items, event.id),
|
||||||
event.tool,
|
event.tool,
|
||||||
event.input,
|
event.input,
|
||||||
"",
|
"",
|
||||||
@@ -262,6 +305,7 @@ fun foldEvent(items: List<TranscriptItem>, entry: SeqEvent): List<TranscriptItem
|
|||||||
TranscriptItem.ToolRun(
|
TranscriptItem.ToolRun(
|
||||||
entry.seq,
|
entry.seq,
|
||||||
event.id,
|
event.id,
|
||||||
|
runIdFor(items, event.id),
|
||||||
"tool",
|
"tool",
|
||||||
"",
|
"",
|
||||||
event.output,
|
event.output,
|
||||||
@@ -896,7 +940,7 @@ fun SessionScreen(
|
|||||||
// reason: the working indicator appearing and disappearing is another insertion
|
// reason: the working indicator appearing and disappearing is another insertion
|
||||||
// at the same end. Paging older history is the opposite insertion and was
|
// at the same end. Paging older history is the opposite insertion and was
|
||||||
// already fine, and stays fine, because a key survives both.
|
// already fine, and stays fine, because a key survives both.
|
||||||
items(rows.asReversed(), key = { it.seq }) { row ->
|
items(rows.asReversed(), key = { it.key }) { row ->
|
||||||
when (row) {
|
when (row) {
|
||||||
is TranscriptRow.Tools ->
|
is TranscriptRow.Tools ->
|
||||||
ToolGroup(
|
ToolGroup(
|
||||||
|
|||||||
@@ -32,26 +32,34 @@ import androidx.compose.ui.unit.dp
|
|||||||
*/
|
*/
|
||||||
sealed class TranscriptRow {
|
sealed class TranscriptRow {
|
||||||
/**
|
/**
|
||||||
* This row's identity in the list, taken from the first transcript event behind it.
|
* This row's identity in the list, which must survive everything that can happen to the row.
|
||||||
*
|
*
|
||||||
* See [TranscriptItem.seq]: the list is keyed by this so that inserting a new message at one
|
* The list is keyed by this so that inserting a new message at one end, or a page of history at
|
||||||
* end, or a page of history at the other, moves the rows and not the reader.
|
* the other, moves the rows and not the reader. That makes it the load-bearing value on this
|
||||||
|
* screen: when a key changes, the list loses its anchor and the transcript steps under whoever
|
||||||
|
* is reading it.
|
||||||
|
*
|
||||||
|
* A tool row therefore keys on [TranscriptItem.ToolRun.runId] rather than on a sequence number,
|
||||||
|
* 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.
|
||||||
*/
|
*/
|
||||||
abstract val seq: Long
|
abstract val key: Any
|
||||||
|
|
||||||
data class Single(val item: TranscriptItem) : TranscriptRow() {
|
data class Single(val item: TranscriptItem) : TranscriptRow() {
|
||||||
override val seq: Long
|
override val key: Any
|
||||||
get() = item.seq
|
get() = (item as? TranscriptItem.ToolRun)?.runId ?: item.seq
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Two or more calls with nothing between them; drawn as one collapsed card. */
|
/** Two or more calls with nothing between them; drawn as one collapsed card. */
|
||||||
data class Tools(val calls: List<TranscriptItem.ToolRun>) : TranscriptRow() {
|
data class Tools(val calls: List<TranscriptItem.ToolRun>) : TranscriptRow() {
|
||||||
/** Stable across reloads because it is the first call's own id. */
|
/** The run's own name, which every call in it already carries. */
|
||||||
val id: String
|
val id: String
|
||||||
get() = calls.first().id
|
get() = calls.first().runId
|
||||||
|
|
||||||
override val seq: Long
|
override val key: Any
|
||||||
get() = calls.first().seq
|
get() = id
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -75,10 +83,16 @@ fun groupToolRuns(items: List<TranscriptItem>): List<TranscriptRow> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
items.forEach { item ->
|
items.forEach { item ->
|
||||||
if (item is TranscriptItem.ToolRun) run += item
|
// Grouped by the run each call says it belongs to, not by adjacency worked out here.
|
||||||
else {
|
// Adjacency is the same answer most of the time and a worse one at the edges: a call
|
||||||
|
// arriving next to an existing run, or a page of history arriving in front of one, both
|
||||||
|
// change which call is *first*, and a group named after its first member is a different
|
||||||
|
// group every time that happens.
|
||||||
|
if (item is TranscriptItem.ToolRun && (run.isEmpty() || run.first().runId == item.runId)) {
|
||||||
|
run += item
|
||||||
|
} else {
|
||||||
flush()
|
flush()
|
||||||
rows += TranscriptRow.Single(item)
|
if (item is TranscriptItem.ToolRun) run += item else rows += TranscriptRow.Single(item)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
flush()
|
flush()
|
||||||
|
|||||||
Reference in new issue
Block a user