diff --git a/AGENTS.md b/AGENTS.md index 76a6676..8e145c8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -385,7 +385,13 @@ written, and the fold uses that same predicate to decide a reply is settled. subagent directory. **What those tasks are is `GET /sessions/{id}/background`**, listed in the session's right panel above the subagents: runtime state, so it is never persisted and `null` -- not an empty list -- is what a session with - no process answers. An `ambient` task is dropped from both the list and the + no process answers. **A task is drawn as the command it ran, and tapping it + goes to the call that started it** -- both from the transcript rather than + from the provider: a driver reports which tool call its task belongs to and + `Session::background_tasks` resolves that id into a seq and, for a provider + that says nothing (Codex names a terminal by a process id), the command on + the call. The phone travels there with `travelTo`, the same journey a + reopened session makes to put a reader back where they stopped. An `ambient` task is dropped from both the list and the count, on the CLI's own instruction: a live-update watcher is not activity, and counting one leaves a session `waiting` for ever. An adopted CLI is sent a repeated `initialize` to ask for the current set. Reconcile only between turns or at a result boundary: diff --git a/PLAN.md b/PLAN.md index 398e372..1d2ca01 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1156,15 +1156,43 @@ written to a transcript and never kept here, because it is a measurement of a running process and a session with none has nothing to report -- that route answers `null` for one, which the panel says in words rather than drawing as an empty list. Each entry is an id the reader never sees, the provider's own -description, and a kind; `description` is optional because Codex names a -background terminal by a process id, and a card with nothing to say says the -kind instead of dressing the number up as a name. The section is collapsed to +description, a kind, and where the call that started it is. The section is +collapsed to its one-line count by default and pushes the subagents down when opened, and it is refetched whenever the live `backgroundTasks` count moves, since a card for work that has finished is exactly the stale measurement the count was designed not to be. A backgrounded subagent appears in both lists: the background one -because it is running, the subagent one because it has a transcript -- and only -the subagent card opens, because only it has anything to open. +because it is running, the subagent one because it has a transcript -- and the +two cards lead to different places, the subagent's to its own transcript and the +background one's to the call in *this* transcript that started it. + +**A card says what was run, and tapping it goes to where it was run** +(2026-09-20). The first version drew the kind as a second line under the +description, which on a panel of backgrounded commands was "background command" +repeated down the list -- and Codex, which names a terminal by a process id and +gives no description at all, drew that phrase as the *whole* of every card. Both +halves of the answer come from the transcript rather than from the provider: a +driver reports the tool call its task belongs to (Claude's `task_started` +carries the `tool_use_id` beside the `task_id`; Codex's terminal list carries +the `itemId`), and `Session::background_tasks` resolves those ids against the +transcript into a sequence number and, where the provider said nothing, the +command the call was made with. So the panel draws the command, and the kind +shrinks to a mark beside it whose name is what a screen reader is given. + +What tapping does is **go to the call**, opened, rather than show the output +beside the panel. The output is already in that card -- a backgrounded command's +report replaces the launch result there, above -- so a second copy would be a +second version of the same truth, and the card in its place also says what the +session was doing when it started. The journey is the one a reopened session +already makes to put a reader back where they stopped, which is why it is one +function (`travelTo`): page history back until the row is loaded, open the call +and the run it is drawn in, then scroll, with nothing drawn on the way. What a +jump has to do first and a restore never does is **release the held backlog**: +events arriving while the reader is away from the newest end are held rather +than applied, so a task started since they scrolled back is in no row at all -- +found by testing it, where the tap looked like it had done nothing. A task +whose call the transcript does not hold -- an adopted process whose start was +never seen -- has no chevron and does not invite the tap. **An `ambient` task is not background work.** The same level signal carries live-update watchers and housekeeping marked `ambient`, which the CLI says diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt index 515959b..8db9b76 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/Api.kt @@ -322,8 +322,18 @@ data class BackgroundTaskSummary( val description: String?, /** `agent`, `command`, `workflow`, or `other` for a kind this build has not heard of. */ val kind: String, + /** + * Where the call that started this is in the transcript, for the reader who taps the card. + * + * Null is a provider that does not say which call a task belongs to, or a call the transcript + * no longer holds -- the card is then something to read rather than something to open. + */ + val call: CallSite?, ) +/** Where one thing is in a transcript: the sequence number of the event that is it. */ +data class CallSite(val seq: Long) + /** * What [sessionId] has running in the background right now. * @@ -344,6 +354,10 @@ fun fetchBackgroundTasks( description = if (row.isNull("description")) null else row.getString("description"), kind = row.getString("kind"), + call = + row.optJSONObject("call")?.let { call -> + CallSite(seq = call.getLong("seq")) + }, ) } } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt index 7522f2e..59e9eae 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/AppRoot.kt @@ -242,6 +242,11 @@ fun AppRoot( remember(here.summary.id) { mutableIntStateOf(here.summary.backgroundTasks) } + // Where the panel has asked the session screen to put the reader: the + // call a background task was started by. Held here rather than inside + // either, because the two are siblings -- the panel is the one being + // tapped and the transcript is the one that can travel. + var goTo by remember(here.summary.id) { mutableStateOf(null) } // The two panels this session can be pulled aside for: its subagents // from the right, and the whole main screen from the left. Both are here // rather than screens of their own for the same reason the explorer is -- @@ -279,6 +284,13 @@ fun AppRoot( backgroundTasks = backgroundTasks, onClose = close, onOpenSubagent = { screen = here.copy(subagent = it) }, + // Closed with it: what the reader asked to see is under this + // panel, and a panel left open over the answer is the one + // thing the tap cannot have meant. + onOpenCall = { + goTo = it + close() + }, ) }, ) { @@ -293,6 +305,8 @@ fun AppRoot( share = share, onShareTaken = { share = null }, onBackgroundTasks = { backgroundTasks = it }, + goTo = goTo, + onGoToTaken = { goTo = null }, ) } } diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/BackgroundTasks.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/BackgroundTasks.kt index f118762..485acd9 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/BackgroundTasks.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/BackgroundTasks.kt @@ -19,6 +19,10 @@ import androidx.compose.material3.TextButton import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.font.FontFamily +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp /** @@ -40,6 +44,7 @@ fun LazyListScope.backgroundTaskSection( expanded: Boolean, onToggle: () -> Unit, onRetry: () -> Unit, + onOpenCall: (CallSite) -> Unit, ) { if (count == 0) return item(key = "background-heading") { @@ -86,58 +91,102 @@ fun LazyListScope.backgroundTaskSection( ) } else -> - uniqueItems(rows, key = { "background-${it.id}" }) { BackgroundTaskCard(it) } + uniqueItems(rows, key = { "background-${it.id}" }) { task -> + BackgroundTaskCard( + task, + onOpen = task.call?.let { call -> { onOpenCall(call) } }, + ) + } } } } /** - * One background task: what it is doing, and what kind of thing is doing it. + * One background task: what it is doing, drawn as one line with its kind as the mark beside it. * - * Not something to open, unlike the subagent cards below it -- a task is a provider's runtime state - * and has no transcript of its own. A backgrounded agent that does is also in the subagent list, - * under its own name. + * The kind used to be a second line under the words, which on a list of backgrounded commands was + * "background command" repeated down the panel -- and for a provider that names a task by a process + * id it was the *whole* card, so every row said the same two words. A mark carries the same + * difference in a width the text does not have to make room for, and it is the [Glyph]'s + * description that keeps the words for anybody who cannot see it. + * + * [onOpen] is where the call that started this is in the transcript, for the readers who tap it: + * null where the provider never said which call it was, or where that call is no longer in the + * transcript, and the card is then a statement rather than a control. The chevron is what says + * which of the two this is, since a card that quietly does nothing when pressed is worse than one + * that never invited the press. */ @Composable -private fun BackgroundTaskCard(task: BackgroundTaskSummary) { - val kind = backgroundTaskKindLabel(task.kind) +private fun BackgroundTaskCard(task: BackgroundTaskSummary, onOpen: (() -> Unit)?) { + val look = backgroundTaskLook(task.kind) OutlinedCard(Modifier.fillMaxWidth()) { - Column(Modifier.padding(horizontal = 12.dp, vertical = 8.dp)) { - // The kind stands in as the title where the provider gave no description, rather than + Row( + verticalAlignment = Alignment.CenterVertically, + modifier = + Modifier.fillMaxWidth() + .then( + if (onOpen == null) Modifier + else + Modifier.clickable( + onClickLabel = "Show where this started", + onClick = onOpen, + ) + ) + .padding(horizontal = 12.dp, vertical = 10.dp), + ) { + Glyph( + look.glyph, + colour = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.semantics { contentDescription = look.words }, + ) + Spacer(Modifier.width(10.dp)) + // The kind stands in as the words where the provider gave no description, rather than // the id it named the task by: Codex reports a process number, which says nothing to // the person reading and would look like a name somebody chose. + // + // Cut at its tail: what identifies a command is the program at its head, and the long + // ones are exactly the ones being read closely. Text( - task.description ?: kind, - style = MaterialTheme.typography.titleSmall, + task.description ?: look.words, + style = + if (look.mono) + MaterialTheme.typography.bodyMedium.copy(fontFamily = FontFamily.Monospace) + else MaterialTheme.typography.bodyMedium, color = if (task.description == null) MaterialTheme.colorScheme.onSurfaceVariant else LocalContentColor.current, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), ) - if (task.description != null) { - Spacer(Modifier.height(2.dp)) - Text( - kind, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) + if (onOpen != null) { + Spacer(Modifier.width(8.dp)) + Chevron(Pointing.Right) } } } } +/** How one kind of background task is drawn: see [backgroundTaskLook]. */ +private data class TaskLook(val glyph: String, val words: String, val mono: Boolean) + /** - * What a [BackgroundTaskSummary.kind] is called on screen. + * Everything a [BackgroundTaskSummary.kind] decides, answered by one `when`. * - * A kind this build has not heard of is named by what every one of them has in common rather than - * by the nearest word we do know, which would be this screen asserting something the server never - * said. + * One rather than three, which is the rule this screen already learned once with the status word + * and its colour: three `when`s over one set is two of them waiting to miss a member. + * + * A kind this build has not heard of takes the question mark and is named by what every one of them + * has in common. The nearest word or mark we do know -- a robot, a terminal -- would be this screen + * deciding what the server meant by a word it invented after this build shipped. */ -private fun backgroundTaskKindLabel(kind: String) = +private fun backgroundTaskLook(kind: String) = when (kind) { - "agent" -> "subagent" - "command" -> "background command" - "workflow" -> "workflow" - else -> "background task" + // A command is drawn in the face a command is drawn in everywhere else here. + "command" -> TaskLook(COMMAND_GLYPH, "background command", mono = true) + "agent" -> TaskLook(AGENT_GLYPH, "subagent", mono = false) + "workflow" -> TaskLook(WORKFLOW_GLYPH, "workflow", mono = false) + else -> TaskLook(UNKNOWN_GLYPH, "background task", mono = false) } /** diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/NerdIcons.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/NerdIcons.kt index b17a614..0af52bc 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/NerdIcons.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/NerdIcons.kt @@ -146,6 +146,24 @@ val SAVE_GLYPH = glyph(0xF0193) */ val DRAG_GLYPH = glyph(0xF035C) +/** + * `md-console_line` -- a shell prompt: a backgrounded command, in the panel beside the turn. + * + * The four marks here are one set, drawn by `backgroundTaskLook`: they exist because the kind of a + * background task used to be a word on a line of its own, which on a list of commands was the same + * two words down the whole panel. Each keeps its words as the description a screen reader is given. + */ +val COMMAND_GLYPH = glyph(0xF07B7) + +/** `md-robot` -- a subagent: something running that is doing its own reasoning. */ +val AGENT_GLYPH = glyph(0xF06A9) + +/** `md-sitemap` -- a workflow: steps arranged by something other than the agent itself. */ +val WORKFLOW_GLYPH = glyph(0xF04AA) + +/** `md-help_circle_outline` -- a background task of a kind this build has not heard of. */ +val UNKNOWN_GLYPH = glyph(0xF0625) + /** * The size an icon draws at beside a line of text. * 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 50bb87f..044c5ee 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -203,6 +203,17 @@ fun SessionScreen( onBackgroundTasks: (Int) -> Unit = {}, /** Said once [share] has been attached here, so it is not attached again. */ onShareTaken: () -> Unit = {}, + /** + * Somewhere in this transcript to put the reader, asked for from outside the screen: the call a + * background task was started by, tapped in the session's own panel. + * + * The panel is over this screen rather than a screen of its own, so it cannot travel through + * the transcript itself -- and the travelling is this screen's anyway, since only it knows what + * is loaded and how to load the rest. See [travelTo]. + */ + goTo: CallSite? = null, + /** Said once [goTo] has been travelled to, so the same journey is not made twice. */ + onGoToTaken: () -> Unit = {}, /** * Draws this screen read-only, on a subagent's own transcript instead of the session's. * @@ -553,6 +564,18 @@ fun SessionScreen( if (followingNewest && held.isEmpty()) record(entry) else held = held + entry } + /** + * Applies what arrived while the reader was away from the newest end, oldest first. + * + * Everything at once rather than paced out, and emptied before the first of them is recorded, + * since [apply] holds anything landing while a backlog is still outstanding. + */ + fun releaseHeld() { + val backlog = held + held = listOf() + backlog.forEach { record(it) } + } + /** * A press on the transcript that would open or close something, and the one thing every such * press has to check first. @@ -794,6 +817,101 @@ fun SessionScreen( } } + /** + * Puts the reader at [anchor], paging history back until the row holding it is loaded. + * + * Two things travel: reopening a session puts the reader back where they stopped, and tapping a + * background task in the session's panel takes them to the call that started it. They are the + * same journey, and the parts that are easy to get wrong -- how much to ask for, what to do + * about the oldest half-row, waiting for the composition to build the row before turning it + * into an index -- are wrong in the same way for both. + * + * Nothing is drawn while this runs (see [restoring]): opening at the newest end and then + * travelling is exactly the journey a reader must never see. + * + * [openCall] opens the tool call at the anchor, and the run it is drawn inside, on the way + * past. That is what makes a tapped background task land on its own card with its output + * showing, rather than on a shut group the reader then has to find it in. + */ + suspend fun travelTo(anchor: ScrollAnchor, openCall: Boolean = false) { + // Pages until the anchor's row is loaded and has something older behind it. The oldest + // loaded row is a half-row that grows when the page behind it arrives, so anchoring into + // one puts the reader where they were only until that lands. + // + // This terminates because `oldestSeq` walks strictly backwards and the anchor is a seq. + // Keying on the row's *name* instead could not promise that -- a tool run is renamed + // whenever the newest page starts somewhere new, so an anchor on one was never found and + // this paged to the first event of the conversation every time. + while (moreHistory && anchorRow(anchor.seq) == null) { + // The whole span in one request rather than a page at a time. `read_window` counts + // *lines* and a transcript numbers them one per seq, so the distance back to the anchor + // is the number of events to ask for -- and were seqs ever sparse, that difference + // overshoots into older history rather than stopping short. + // + // Capped, and the loop is what makes the cap safe: a span past it comes back in several + // requests instead of one, which is what this did for every restore until now -- + // thirteen sequential round trips to reopen a session somebody had read a little way + // back into. Raw, not coalesced: this counts events back to a known seq, and a page + // measured in rows cannot be counted to one. + val behind = oldestSeq - anchor.seq + val loaded = + if (behind < 0) { + // The anchor's row is loaded but is the oldest half-row, which [anchorRow] + // refuses; what completes it is the row before it, and only a page counted in + // rows can promise to reach that. Counted in events the span is negative and + // was coerced to one: a request per delta, six hundred round trips for an + // anchor inside a 1,400-delta reply. + loadOlderPage() + } else { + loadOlderPage( + (behind + RESTORE_PAGE_CUSHION) + .coerceIn(1L, RESTORE_PAGE_MAX.toLong()) + .toInt(), + coalesce = false, + ) + } + if (!loaded) break + } + // Opened before the anchor is resolved, because opening a call held inside a group is what + // decides how tall that row is and the scroll is aimed at the row. + if (openCall) { + val call = + items.filterIsInstance().firstOrNull { + it.seq == anchor.seq + } + if (call != null) { + // The group it is drawn in, where it is drawn in one: a call inside a shut group is + // open behind a heading, which is nothing at all on screen. Recorded as having been + // in a group at the same moment, which is what the effect below does a composition + // later -- without it, whether opening the call pulls it out of its group depends + // on which of the two got there first. + groupToolRuns(items, heldOut) + .filterIsInstance() + .firstOrNull { group -> group.calls.any { it.id == call.id } } + ?.let { + everGrouped = everGrouped + call.id + expandedGroups = expandedGroups + it.key + } + expandedTools = expandedTools + call.id + } + } + // Resolved to the row that *holds* the position rather than passed straight through, + // because the two are not always the same seq: the events behind a row regroup between the + // save and the reopen. Null is a row no longer in the transcript at all, and means there is + // nothing to put back. + anchorRow(anchor.seq)?.let { rowSeq -> + // The units are built by composition, and this coroutine has been loading rows the + // composition may not have seen -- so wait for the build that holds the anchor's row + // before turning it into an index. Guaranteed to arrive, because the units are a pure + // function of `items`. Nothing is drawn during the wait. One past the index, because + // item zero is the "below" slot. + val index = + snapshotFlow { unitIndexFor(currentUnits, rowSeq, anchor.unit) } + .first { it != null }!! + listState.scrollToItem(index + 1, anchor.offset) + } + } + // Which calls have been inside a group, which is what [heldOut] subtracts: a call the reader // opened while it stood on its own is held out of the run it belongs to until they close it, // and a call that has been in a group is one that opening will never take back out. @@ -901,62 +1019,7 @@ fun SessionScreen( try { // Then back where reading stopped. An anchor deeper than the newest page is exactly the // one worth restoring -- somebody who read to the bottom has no anchor at all. - savedAnchor?.let { anchor -> - // Pages until the anchor's row is loaded and has something older behind it. The - // oldest loaded row is a half-row that grows when the page behind it arrives, so - // anchoring into one puts the reader where they were only until that lands. - // - // This terminates because `oldestSeq` walks strictly backwards and the anchor is a - // seq. Keying on the row's *name* instead could not promise that -- a tool run is - // renamed whenever the newest page starts somewhere new, so an anchor on one was - // never found and this paged to the first event of the conversation every time. - while (moreHistory && anchorRow(anchor.seq) == null) { - // The whole span in one request rather than a page at a time. `read_window` - // counts *lines* and a transcript numbers them one per seq, so the distance - // back to the anchor is the number of events to ask for -- and were seqs ever - // sparse, that difference overshoots into older history rather than stopping - // short. - // - // Capped, and the loop is what makes the cap safe: a span past it comes back in - // several requests instead of one, which is what this did for every restore - // until now -- thirteen sequential round trips to reopen a session somebody had - // read a little way back into. Raw, not coalesced: this counts events back to a - // known seq, and a page measured in rows cannot be counted to one. - val behind = oldestSeq - anchor.seq - val loaded = - if (behind < 0) { - // The anchor's row is loaded but is the oldest half-row, which - // [anchorRow] refuses; what completes it is the row before it, and only - // a page counted in rows can promise to reach that. Counted in events - // the span is negative and was coerced to one: a request per delta, six - // hundred round trips for an anchor inside a 1,400-delta reply. - loadOlderPage() - } else { - loadOlderPage( - (behind + RESTORE_PAGE_CUSHION) - .coerceIn(1L, RESTORE_PAGE_MAX.toLong()) - .toInt(), - coalesce = false, - ) - } - if (!loaded) break - } - // Resolved to the row that *holds* the saved position rather than passed straight - // through, because the two are not always the same seq: the events behind a row - // regroup between the save and the reopen. Null is a row no longer in the - // transcript at all, and means there is nothing to put back. - anchorRow(anchor.seq)?.let { rowSeq -> - // The units are built by composition, and this coroutine has been loading rows - // the composition may not have seen -- so wait for the build that holds the - // anchor's row before turning it into an index. Guaranteed to arrive, because - // the units are a pure function of `items`. Nothing is drawn during the wait. - // One past the index, because item zero is the "below" slot. - val index = - snapshotFlow { unitIndexFor(currentUnits, rowSeq, anchor.unit) } - .first { it != null }!! - listState.scrollToItem(index + 1, anchor.offset) - } - } + savedAnchor?.let { travelTo(it) } } catch (e: ApiException) { // A page of history that never arrived. The reader is left at the newest end rather // than where they were, which is the state this screen opens in anyway. @@ -982,6 +1045,34 @@ fun SessionScreen( } } + // Where the session's panel asked for the reader to be put. Gated on `ready`, which is the + // opening effect having finished putting them wherever they were: the two both scroll, and a + // jump that won the race would be undone by the restore landing behind it. + LaunchedEffect(goTo, ready, epoch) { + val target = goTo ?: return@LaunchedEffect + if (!ready) return@LaunchedEffect + // What arrived while the reader was back here is in no row at all -- events landing away + // from the newest end are held rather than applied -- and a call started since is exactly + // the one being tapped. So the backlog lands before the journey is planned; the reader is + // about to be moved deliberately anyway, which is the thing holding it exists to avoid. + releaseHeld() + // And whatever arrives from here on is held again: the reader asked to be somewhere, and + // the newest end is not it. + followingNewest = false + restoring = true + try { + travelTo(ScrollAnchor(target.seq, offset = 0), openCall = true) + } catch (e: ApiException) { + // The history the journey needed never arrived, so the reader is where they already + // were. Said rather than silently ignored -- a tap that did nothing at all is a broken + // control. + streamError = e.message + } finally { + restoring = false + onGoToTaken() + } + } + // Only while the screen is actually on screen. Android stops the activity when somebody // switches away and the socket dies with it, which arrived as "Lost the event stream" waiting // at the top on their return. Switching apps is a choice somebody made, not a fault to report. @@ -1094,19 +1185,15 @@ fun SessionScreen( } } - // Back at the newest end, so the backlog [apply] held can land. Everything at once rather than - // paced out: they are at the bottom, which is the one place the list is allowed to follow new - // content. + // Back at the newest end, so the backlog held can land: the bottom is the one place the list is + // allowed to follow new content. LaunchedEffect(listState) { snapshotFlow { Triple(listState.isScrollInProgress, atNewest, held.isNotEmpty()) } .collect { (scrolling, newest, hasHeld) -> if (scrolling && !newest) followingNewest = false if (!newest) return@collect followingNewest = true - if (!hasHeld) return@collect - val backlog = held - held = listOf() - backlog.forEach { record(it) } + if (hasHeld) releaseHeld() } } // Where the reader left off, written whenever the list settles somewhere new. diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SubagentPanel.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SubagentPanel.kt index 6441b83..2ff0abe 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SubagentPanel.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SubagentPanel.kt @@ -45,6 +45,9 @@ import kotlinx.coroutines.withContext * [active] is whether the panel is being looked at: the lists are fetched then rather than on * composition, since the panel is composed for every session whether or not anybody opens it. * + * [onOpenCall] takes the reader to where a background task was started, in the transcript under + * this panel -- so the panel is closed with it, which is the caller's to do. + * * [backgroundTasks] is the live count from the session's own event stream, and is what the * background list is refetched against: a card for work that has since finished is a stale * measurement drawn as a current one, which is the one thing a list of what is running now must not @@ -61,6 +64,7 @@ fun SubagentPanel( backgroundTasks: Int, onClose: () -> Unit, onOpenSubagent: (SubagentSummary) -> Unit, + onOpenCall: (CallSite) -> Unit, ) { val scope = rememberCoroutineScope() val context = LocalContext.current @@ -128,6 +132,7 @@ fun SubagentPanel( expanded = backgroundExpanded, onToggle = { backgroundExpanded = !backgroundExpanded }, onRetry = { refreshToken++ }, + onOpenCall = onOpenCall, ) item(key = "subagents-heading") { PanelSectionHeading("Subagents") } when (val state = rows) { diff --git a/app/androidApp/src/main/res/font/nerd_icons.ttf b/app/androidApp/src/main/res/font/nerd_icons.ttf index ba5d5c1..cae3644 100644 Binary files a/app/androidApp/src/main/res/font/nerd_icons.ttf and b/app/androidApp/src/main/res/font/nerd_icons.ttf differ diff --git a/app/build-icon-font.sh b/app/build-icon-font.sh index 2a8fc88..876a857 100755 --- a/app/build-icon-font.sh +++ b/app/build-icon-font.sh @@ -46,6 +46,10 @@ GLYPHS=( U+F0224 # md-file_outline U+F201 # fa-line_chart -- Font Awesome's, asked for by name U+F035C # md-menu -- the burger, as a row's drag handle + U+F07B7 # md-console_line -- a backgrounded command + U+F06A9 # md-robot -- a subagent + U+F04AA # md-sitemap -- a workflow + U+F0625 # md-help_circle_outline -- a background task of a kind this build does not know ) url=https://github.com/ryanoasis/nerd-fonts/releases/latest/download/NerdFontsSymbolsOnly.zip diff --git a/server/src/routes.rs b/server/src/routes.rs index 4f2034d..d40a7e0 100644 --- a/server/src/routes.rs +++ b/server/src/routes.rs @@ -49,8 +49,10 @@ //! ?limit=N, ?coalesce=true to count rows not deltas, //! ?after=N to floor it at what the caller already holds //! GET /sessions/{id}/background what it has running in the background right now: -//! [{id, description?, kind}], or null when its provider -//! has not said -- runtime state, never a transcript row +//! [{id, description?, kind, call?: {seq}}], or null when +//! its provider has not said -- runtime state, never a +//! transcript row. `call` is where the tool call that +//! started it is in the transcript //! GET /sessions/{id}/subagents [{id, title, status, created, lastActivity}], oldest //! first -- see SUBAGENTS.md //! GET /sessions/{id}/subagents/{sub}/transcript exactly the transcript route above, @@ -124,11 +126,11 @@ use tokio::sync::{broadcast, mpsc}; use tokio_stream::StreamExt; use tokio_stream::wrappers::{BroadcastStream, ReceiverStream}; -use crate::session::driver::{BackgroundTask, SessionCommand, Unqueued}; +use crate::session::driver::{SessionCommand, Unqueued}; use crate::session::pending::Operation; use crate::session::subagent::{Subagent, SubagentInfo}; use crate::session::transcript::{CATCH_UP_LIMIT, CatchUp, SeqEvent, catch_up}; -use crate::session::{LiveSession, SessionInfo, SessionManager, SpawnSpec}; +use crate::session::{BackgroundTaskView, LiveSession, SessionInfo, SessionManager, SpawnSpec}; pub fn router(manager: Arc) -> Router { Router::new() @@ -2387,13 +2389,13 @@ fn sse_stream( } /// `GET /sessions/{id}/background`: the provider's own snapshot of what this -/// session has running -- see [`BackgroundTask`]. `null` is "nobody has +/// session has running -- see [`BackgroundTaskView`]. `null` is "nobody has /// said", which a session with no process answers, rather than "there is /// none". async fn list_background_tasks( State(manager): State>, UrlPath(id): UrlPath, -) -> Result>>, ApiError> { +) -> Result>>, ApiError> { Ok(axum::Json(lookup(&manager, &id)?.background_tasks())) } diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 15f48e5..5139761 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -674,19 +674,28 @@ impl Translator { .and_then(Value::as_bool) .unwrap_or(false) }) - .map(|task| BackgroundTask { - id: task + .map(|task| { + let id = task .get("task_id") .and_then(Value::as_str) .unwrap_or_default() - .to_string(), - description: text_field(task, "description"), - kind: match task.get("task_type").and_then(Value::as_str) { - Some("local_agent") => BackgroundTaskKind::Agent, - Some("local_bash" | "local_shell") => BackgroundTaskKind::Command, - Some("local_workflow") => BackgroundTaskKind::Workflow, - _ => BackgroundTaskKind::Other, - }, + .to_string(); + BackgroundTask { + description: text_field(task, "description"), + kind: match task.get("task_type").and_then(Value::as_str) { + Some("local_agent") => BackgroundTaskKind::Agent, + Some("local_bash" | "local_shell") => BackgroundTaskKind::Command, + Some("local_workflow") => BackgroundTaskKind::Workflow, + _ => BackgroundTaskKind::Other, + }, + // The level's own ids are not promised to mean anything + // outside it, so the call is the one `task_started` + // carried for this task -- and a task whose start this + // translator never saw, an adopted process's, simply has + // none. + call: self.tasks.get(&id).cloned(), + id, + } }) .collect(); let count = live.len(); @@ -1959,11 +1968,27 @@ mod tests { /// The panel lists these, so each entry is read rather than counted -- /// and an `ambient` one is dropped from the list and the count alike, /// since a watcher counted as work leaves a session `waiting` for ever. + /// + /// Each task also carries the call that started it, where this translator + /// saw the `task_started` that named one: that is what the reader is + /// taken to on tapping the card, and the level's own ids mean nothing + /// outside it. A task whose start was never seen -- `t2` here -- has + /// none, and is still listed. #[test] fn a_background_snapshot_describes_each_task_and_drops_ambient_ones() { let dir = tempfile::tempdir().expect("tempdir"); let mut translator = Translator::new(dir.path().to_path_buf(), test_subagents(&dir)); + let started = json!({ + "type": "system", + "subtype": "task_started", + "task_id": "t1", + "tool_use_id": "toolu_one", + "task_type": "local_bash", + }) + .to_string(); + assert_eq!(translate_lines(&mut translator, &[&started]), Vec::new()); + let line = json!({ "type": "system", "subtype": "background_tasks_changed", @@ -1985,11 +2010,13 @@ mod tests { id: "t1".to_string(), description: Some("run the tests".to_string()), kind: BackgroundTaskKind::Command, + call: Some("toolu_one".to_string()), }, BackgroundTask { id: "t2".to_string(), description: Some("review the diff".to_string()), kind: BackgroundTaskKind::Agent, + call: None, }, ]) ); diff --git a/server/src/session/codex.rs b/server/src/session/codex.rs index 84ea499..842c308 100644 --- a/server/src/session/codex.rs +++ b/server/src/session/codex.rs @@ -9,7 +9,7 @@ mod translate; -use std::collections::{HashSet, VecDeque}; +use std::collections::{BTreeMap, VecDeque}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; @@ -38,7 +38,11 @@ const STATE_FILE: &str = "codex-state.json"; const POLL: std::time::Duration = std::time::Duration::from_millis(50); const BACKGROUND_POLL: std::time::Duration = std::time::Duration::from_secs(1); -type BackgroundProcesses = Arc>>; +/// The background terminals app-server says are alive, by process id, each +/// against the transcript item whose call started it where the list named +/// one. The item id is what the panel draws a command rather than a process +/// number for, and what takes a reader to the call. +pub(super) type BackgroundProcesses = Arc>>>; #[derive(Default)] struct BackgroundQuery { @@ -49,7 +53,7 @@ struct BackgroundQuery { struct PendingBackgroundQuery { request: String, thread: String, - found: HashSet, + found: BTreeMap>, dirty: bool, } @@ -194,7 +198,7 @@ impl CodexDriver { } }); - let background_processes = Arc::new(Mutex::new(HashSet::new())); + let background_processes = Arc::new(Mutex::new(BTreeMap::new())); let inner = Arc::new(Inner { sink, state: Mutex::new(state), @@ -672,16 +676,25 @@ pub(super) fn background_tasks( id, description: Some(title), kind: BackgroundTaskKind::Agent, + // A subagent is opened from the list below this one, which is + // where its own transcript is read; the call that spawned it is + // not what a reader of this row wants. + call: None, }) .collect(); - let mut running: Vec = processes.lock().unwrap().iter().cloned().collect(); - running.sort(); - tasks.extend(running.into_iter().map(|id| BackgroundTask { + // In process-id order, which is the order the map holds them in: the list + // itself is a snapshot with no order of its own, and two fetches must not + // disagree about how it reads. + let running = processes.lock().unwrap().clone(); + tasks.extend(running.into_iter().map(|(id, item)| BackgroundTask { id, // The terminal list carries a process id and no name, and a number // drawn as a name is worse than the panel saying it does not know. + // What the panel draws instead is the command, resolved from the item + // below by whoever answers the route. description: None, kind: BackgroundTaskKind::Command, + call: item, })); tasks } @@ -707,7 +720,7 @@ fn refresh_background_terminals(inner: &Inner) { query.pending = Some(PendingBackgroundQuery { request: request.clone(), thread: thread.clone(), - found: HashSet::new(), + found: BTreeMap::new(), dirty: false, }); request @@ -755,11 +768,17 @@ fn handle_background_response(inner: &Inner, line: &Value) -> bool { query.pending = None; return true; } - for process in terminals - .iter() - .filter_map(|terminal| terminal.get("processId").and_then(Value::as_str)) - { - pending.found.insert(process.to_string()); + for terminal in terminals { + let Some(process) = terminal.get("processId").and_then(Value::as_str) else { + continue; + }; + pending.found.insert( + process.to_string(), + terminal + .get("itemId") + .and_then(Value::as_str) + .map(str::to_string), + ); } if let Some(cursor) = line.pointer("/result/nextCursor").and_then(Value::as_str) { let thread = pending.thread.clone(); @@ -1582,7 +1601,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }; @@ -1598,7 +1617,7 @@ mod tests { "id": request_id, "result": { "data": [ - {"processId": "process-a"}, + {"processId": "process-a", "itemId": "item-a"}, {"processId": "process-b"} ], "nextCursor": null @@ -1606,6 +1625,26 @@ mod tests { }) )); assert_eq!(background_task_count(&inner), 2); + // The item the terminal belongs to is what the panel draws a command + // rather than a process number for, so it is kept beside the id; + // a terminal listed without one is still a task. + assert_eq!( + background_tasks(&inner.subagents, &inner.background_processes), + vec![ + BackgroundTask { + id: "process-a".to_string(), + description: None, + kind: BackgroundTaskKind::Command, + call: Some("item-a".to_string()), + }, + BackgroundTask { + id: "process-b".to_string(), + description: None, + kind: BackgroundTaskKind::Command, + call: None, + }, + ] + ); assert_eq!( events.try_recv().expect("count"), Event::BackgroundTasks { count: 2 } @@ -1691,7 +1730,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }); @@ -1748,7 +1787,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }); @@ -1815,7 +1854,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }); @@ -1880,7 +1919,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }); @@ -1951,7 +1990,7 @@ mod tests { transport: Transport::Here, session_dir: dir.path().to_path_buf(), subagents: Arc::new(Subagents::new(dir.path().to_path_buf())), - background_processes: Arc::new(Mutex::new(HashSet::new())), + background_processes: Arc::new(Mutex::new(BTreeMap::new())), background_query: Mutex::new(BackgroundQuery::default()), reading: AtomicBool::new(true), }); diff --git a/server/src/session/codex/translate.rs b/server/src/session/codex/translate.rs index b28a1c6..52a2d2d 100644 --- a/server/src/session/codex/translate.rs +++ b/server/src/session/codex/translate.rs @@ -5,7 +5,7 @@ //! ignore the rest; an added Codex item must not make a live session go deaf. use std::collections::{HashMap, HashSet}; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use serde_json::{Value, json}; @@ -18,7 +18,7 @@ pub(super) struct Translator { completed: bool, limited: bool, subagents: Option>, - background_processes: Option>>>, + background_processes: Option, children: HashMap, prompts: HashMap, async_messages: HashSet, @@ -28,7 +28,7 @@ pub(super) struct Translator { impl Translator { pub(super) fn new( subagents: Arc, - background_processes: Arc>>, + background_processes: super::BackgroundProcesses, thread_id: Option, in_turn: bool, ) -> Self { @@ -912,6 +912,9 @@ fn find_reset(value: &Value) -> Option { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use std::sync::Mutex; + use super::*; fn line(text: &str) -> Value { @@ -1231,7 +1234,7 @@ mod tests { fn codex_subagents_get_their_own_transcripts_and_hold_the_parent_waiting() { let dir = tempfile::tempdir().expect("tempdir"); let subagents = Arc::new(Subagents::new(dir.path().to_path_buf())); - let background_processes = Arc::new(Mutex::new(HashSet::new())); + let background_processes = Arc::new(Mutex::new(BTreeMap::new())); let mut translator = Translator::new( Arc::clone(&subagents), Arc::clone(&background_processes), @@ -1330,7 +1333,7 @@ mod tests { fn codex_reports_each_change_to_its_live_background_count() { let dir = tempfile::tempdir().expect("tempdir"); let subagents = Arc::new(Subagents::new(dir.path().to_path_buf())); - let background_processes = Arc::new(Mutex::new(HashSet::new())); + let background_processes = Arc::new(Mutex::new(BTreeMap::new())); let mut translator = Translator::new( Arc::clone(&subagents), Arc::clone(&background_processes), @@ -1357,7 +1360,7 @@ mod tests { background_processes .lock() .unwrap() - .insert("command-a".to_string()); + .insert("command-a".to_string(), None); for (child, count) in [("child-a", 2), ("child-b", 1)] { let events = translator.translate(&json!({ "method": "item/completed", @@ -1521,7 +1524,7 @@ mod tests { subagents.start("child-thread", "child", Some("work")); let mut translator = Translator::new( Arc::clone(&subagents), - Arc::new(Mutex::new(HashSet::new())), + Arc::new(Mutex::new(BTreeMap::new())), Some("parent-thread".to_string()), true, ); diff --git a/server/src/session/driver.rs b/server/src/session/driver.rs index 6d34060..bd0ce93 100644 --- a/server/src/session/driver.rs +++ b/server/src/session/driver.rs @@ -791,21 +791,28 @@ pub type EventSink = mpsc::UnboundedSender; /// leave going. /// /// Runtime state, never written to a transcript: it is what the provider -/// says right now, so a session with no process has nothing to say. Served -/// by `GET /sessions/{id}/background`; the `BackgroundTasks` event carries -/// only the size, which is what the status row draws. +/// says right now, so a session with no process has nothing to say. What +/// `GET /sessions/{id}/background` answers is this resolved against the +/// transcript -- see [`BackgroundTaskView`](crate::session::BackgroundTaskView); +/// the `BackgroundTasks` event carries only the size, which is what the +/// status row draws. /// /// [`description`](Self::description) is `None` where the provider names a /// task by something no reader would recognise -- a process id -- rather -/// than by a sentence worked out here; the phone says it does not know. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +/// than by a sentence worked out here. Where [`call`](Self::call) is known +/// the transcript answers it instead, from the call's own arguments; see +/// [`LiveSession::background_tasks`](crate::session::LiveSession::background_tasks). +#[derive(Debug, Clone, PartialEq, Eq)] pub struct BackgroundTask { /// The provider's own id for it. Never shown; it is what makes two /// snapshots comparable, and what keys the list on the phone. pub id: String, - #[serde(default, skip_serializing_if = "Option::is_none")] pub description: Option, pub kind: BackgroundTaskKind, + /// The id of the tool call that started it, where the provider says + /// which. `None` is a provider that does not, or one whose account of + /// the start was never seen -- an adopted process mid-task. + pub call: Option, } /// What kind of thing a [`BackgroundTask`] is, in the terms the app draws. diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index bc21cef..2219069 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -509,8 +509,13 @@ impl EchoDriver { let mut tasks = background_tasks.lock().unwrap(); tasks.push(BackgroundTask { id: id.clone(), - description: Some(command.clone()), + // Deliberately unsaid, though this driver knows it: the + // command is on the call above, and leaving it to be + // resolved from there is what makes `/background` exercise + // the path a real provider's process id takes. + description: None, kind: BackgroundTaskKind::Command, + call: Some(id.clone()), }); self.emit(Event::BackgroundTasks { count: tasks.len() }); } diff --git a/server/src/session/mod.rs b/server/src/session/mod.rs index 5df98cb..b9279c0 100644 --- a/server/src/session/mod.rs +++ b/server/src/session/mod.rs @@ -20,7 +20,7 @@ pub mod subagent; pub mod transcript; pub mod transport; -use std::collections::{BTreeMap, HashMap, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, RwLock}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -36,8 +36,8 @@ use crate::config::{ use claude::ClaudeDriver; use codex::CodexDriver; use driver::{ - AttachmentRef, BackgroundTask, Driver, Event, EventSink, Images, SessionCommand, SessionStatus, - Unqueued, context_after, context_limit_after, + AttachmentRef, BackgroundTaskKind, Driver, Event, EventSink, Images, SessionCommand, + SessionStatus, Unqueued, context_after, context_limit_after, }; use echo::EchoDriver; use llama::LlamaDriver; @@ -170,6 +170,48 @@ fn resume_message(meta: &SessionConfig) -> String { .unwrap_or_else(|| DEFAULT_RESUME_MESSAGE.to_string()) } +/// One background task as `GET /sessions/{id}/background` answers it: what +/// the provider said about it, plus where the transcript says it began. +/// +/// A view rather than a [`driver::BackgroundTask`] with more fields on it, because +/// the two are answered by different things -- a driver knows what is +/// running and cannot know where a sequence number is, and only this side +/// has read the transcript. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct BackgroundTaskView { + pub id: String, + /// What it is doing: the provider's own words, or -- for one that names + /// a task by a process id -- the command its call was made with. + #[serde(skip_serializing_if = "Option::is_none")] + pub description: Option, + pub kind: BackgroundTaskKind, + /// Where in the transcript this was started, for the reader who taps it. + /// Absent where the call is not in the transcript, or where the provider + /// never said which call it was. + #[serde(skip_serializing_if = "Option::is_none")] + pub call: Option, +} + +/// Where one tool call is. A struct of one field, so that adding what a +/// caller needs next does not mean another optional beside `seq` that is +/// only meaningful when it is set. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct CallSite { + pub seq: u64, +} + +/// The command a call was made with, where it was made with one -- every way +/// a backgrounded command reaches a transcript here (`Bash`, Codex's `Shell`) +/// has the whole of it under `command`. A call with no such argument keeps +/// its `None` rather than being described by some other one it happens to +/// have, which would read as a command that was never run. +fn command_of(input: &serde_json::Value) -> Option { + let command = input.get("command")?.as_str()?.trim(); + (!command.is_empty()).then(|| command.to_string()) +} + /// One row of `GET /sessions`. #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] @@ -528,8 +570,41 @@ impl LiveSession { /// What this session has running in the background, as its provider last /// said -- `None` when nothing has said, which includes a session with no /// process. Serves `GET /sessions/{id}/background`. - pub fn background_tasks(&self) -> Option> { - self.driver().and_then(|driver| driver.background_tasks()) + /// + /// Each task is resolved against the transcript here rather than in the + /// driver, because both things the resolution produces are facts about + /// the transcript and not about the provider: where the call that started + /// the task is, which is where tapping the card takes the reader, and -- + /// for a provider that names a task by a process id -- the arguments that + /// call was made with, which is the command the panel draws instead of + /// "background task" repeated down the list. + pub fn background_tasks(&self) -> Option> { + let tasks = self.driver().and_then(|driver| driver.background_tasks())?; + let wanted: HashSet = tasks.iter().filter_map(|task| task.call.clone()).collect(); + // A transcript that cannot be read costs the calls, not the list: the + // tasks themselves are what the panel is for, and they are all still + // here. + let calls = + transcript::locate_tool_calls(&self.transcript_path, &wanted).unwrap_or_else(|err| { + tracing::warn!("locating background tasks' calls: {err:#}"); + HashMap::new() + }); + Some( + tasks + .into_iter() + .map(|task| { + let found = task.call.as_deref().and_then(|id| calls.get(id)); + BackgroundTaskView { + description: task + .description + .or_else(|| found.and_then(|call| command_of(&call.input))), + call: found.map(|call| CallSite { seq: call.seq }), + id: task.id, + kind: task.kind, + } + }) + .collect(), + ) } /// What this session is doing right now, as the pump last recorded it -- diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index a211299..8393f91 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -6,6 +6,7 @@ //! Reopening an existing file continues the numbering, which is what makes //! a backend restart invisible to a phone holding a cursor. +use std::collections::{HashMap, HashSet}; use std::fs::{File, OpenOptions}; use std::io::Write; use std::ops::Range; @@ -240,6 +241,59 @@ pub fn read_after(path: &Path, after: u64) -> Result> { indexed.parse(start..indexed.lines.len()) } +/// Where each of `ids` was called, for the ids that are in the transcript: +/// the call's sequence number and the arguments it was made with. +/// +/// What this is for is a background task: a provider names the tool call that +/// started one, and both the words the panel draws it with and the place the +/// reader is taken on tapping it come from the call itself. Nothing else here +/// answers "where is this id", because nothing else needed to -- every other +/// reader of a transcript wants a range of it. +/// +/// Walked newest-first and stopped as soon as every id is found, since a task +/// that is still running was started recently: the whole file is only parsed +/// for an id that is not in it at all. The substring test before each parse is +/// what keeps that worst case a scan of the text rather than 24,000 parses. +pub fn locate_tool_calls(path: &Path, ids: &HashSet) -> Result> { + let mut found = HashMap::new(); + if ids.is_empty() { + return Ok(found); + } + let Some(indexed) = Indexed::read(path)? else { + return Ok(found); + }; + for index in (0..indexed.lines.len()).rev() { + let line = &indexed.text[indexed.lines[index].clone()]; + if !ids.iter().any(|id| line.contains(id.as_str())) { + continue; + } + let entry = indexed.parse_one(index)?; + let Event::ToolStart { id, input, .. } = entry.event else { + continue; + }; + if !ids.contains(&id) { + continue; + } + found.insert( + id, + ToolCall { + seq: entry.seq, + input, + }, + ); + if found.len() == ids.len() { + break; + } + } + Ok(found) +} + +/// One tool call as [`locate_tool_calls`] found it. +pub struct ToolCall { + pub seq: u64, + pub input: serde_json::Value, +} + /// The transcript's lines located but not read, so a reader can find the range /// it wants and parse only that. /// @@ -586,6 +640,53 @@ mod tests { assert!(read_after(&path, 3).expect("read").is_empty()); } + /// What a background task's card is drawn from and what tapping it moves + /// to. An id that was never a tool call -- or one whose call is a kind + /// this never recorded arguments for -- is simply absent, which is the + /// card with nothing to go to rather than an error. + #[test] + fn a_tool_call_is_found_by_its_id_and_a_stranger_is_not() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("transcript.jsonl"); + let mut transcript = Transcript::open(&path).expect("open"); + transcript.append(text("before"), 1.0).expect("append"); + for id in ["call-one", "call-two"] { + transcript + .append( + Event::ToolStart { + id: id.to_string(), + tool: "Bash".to_string(), + input: serde_json::json!({"command": format!("run {id}")}), + }, + 2.0, + ) + .expect("append"); + } + transcript.append(text("after"), 3.0).expect("append"); + + let wanted = ["call-two", "never-called"] + .into_iter() + .map(str::to_string) + .collect(); + let found = locate_tool_calls(&path, &wanted).expect("locate"); + assert_eq!(found.len(), 1); + let call = found.get("call-two").expect("the call that was made"); + assert_eq!(call.seq, 3); + assert_eq!(call.input["command"], "run call-two"); + + assert!( + locate_tool_calls(&path, &HashSet::new()) + .expect("locate") + .is_empty() + ); + let missing = dir.path().join("not-a-transcript.jsonl"); + assert!( + locate_tool_calls(&missing, &wanted) + .expect("locate") + .is_empty() + ); + } + #[test] fn reopening_continues_the_numbering() { let dir = tempfile::tempdir().expect("tempdir");