mirror of
https://github.com/block/buzz.git
synced 2026-11-03 07:51:22 +00:00
fix(desktop): guard the work-block pulse and stop a finished block going live
Two reader-visible bugs in the focus-mode work block, both found reviewing this PR rather than by a test. **The running step's pulse ignored reduced motion.** `WorkBlockRailGlyph` applied bare `animate-pulse`. `useWorkBlockMotionEnabled` skips the fold's height animation, but it cannot reach a keyframe animation applied by a utility class, and none of the app's 20 `prefers-reduced-motion: reduce` blocks matches `.animate-pulse` — every one is scoped to a `buzz-*`/`motion-*`/`t-skel-*` class. So a reader who asked for no motion got an indefinite pulse. Now `motion-safe:animate-pulse`, which this repo's Tailwind (4.3.0) compiles to the same declaration wrapped in `@media (prefers-reduced-motion: no-preference)` — and which matches every other pulse in this feature (`AgentStatusBadge`, `ManagedAgentRow`). That swap also repaired three assertions it would otherwise have disarmed. `motion-safe:animate-pulse` is a different class TOKEN, not `animate-pulse` plus a modifier, so the suite's `.animate-pulse` selectors would have stopped matching anything and passed unconditionally. Worse, they were already vacuous: all three ran after `settleToStepCount(0)`, i.e. on a folded rail with no rows, so "nothing pulses" held against a build where abandoned steps pulse forever — the exact Codex finding this PR answers. They now assert on rows that exist (expanded, or mid-settle) and match the exact class token, so neither a substring nor a missing element can fake a pass. **A finished block went live again between turns.** `lastTurnId` walked the display blocks for the newest `turn` block, but a turn that has only emitted setup lifecycle rows (`turn_started`, `session_resolved`) classifies to zero segments and so produces no block at all. For the whole gap between `turn_started` and the next turn's first renderable item, "newest turn with a block" was therefore the turn that had already ENDED, and its own trailing item became the streaming item: a settled 6-step block re-opened, dropped to its last three steps behind a previous-steps disclosure, then folded back. `turn_started` fires on every turn and the gap is real observer-stream latency, so this was every turn, not an edge case. Liveness now comes from the newest turn id in the item stream, which is why `buildConversationTurnMeta` takes `items`. A trailing item is also only reported as streaming when the live turn owns it — the same ownership rule `toolEntryState` already applies to an `executing` status, for the same reason: without it the newest block's tail holds a finished turn's block open regardless of whose turn it is. Tests, each mutation-checked in isolation so none of them is vacuous: - pulse guarded, asserted under BOTH preference values (the class must not depend on it) — reverting to the bare class fails exactly that test; dropping the `running` guard fails the three retargeted negatives. - the meta gap contract over the real 4-frame sequence built from a raw item stream through the real grouping, since the bug is precisely which turns do and do not produce a block — reverting either half of the fix fails it. - the rendered consequence with a 6-step block (above the live window, where the symptom is loudest): folded summary and no rail across the gap. Also fixed a fixture that named the wrong thing: "no turn at all" carried the shared helper's default `turnId: "turn-1"`, so it was asserting that a turn with no block reports no live turn — the opposite of the rule. Full desktop suite 5491 passing / 0 failing, 81 suites; `tsc --noEmit` clean. Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
@@ -176,8 +176,9 @@ export function AgentSessionTranscriptList({
|
||||
// Returns a shared constant for the other variants, so their render output is
|
||||
// untouched.
|
||||
const turnMeta = React.useMemo(
|
||||
() => buildConversationTurnMeta(displayBlocks, { isTurnLive, variant }),
|
||||
[displayBlocks, isTurnLive, variant],
|
||||
() =>
|
||||
buildConversationTurnMeta(displayBlocks, { isTurnLive, items, variant }),
|
||||
[displayBlocks, isTurnLive, items, variant],
|
||||
);
|
||||
|
||||
const scrollContainerClassNames = cn(
|
||||
|
||||
@@ -45,9 +45,22 @@ test("a block whose running step has no live session folds like finished work",
|
||||
"an abandoned step is not known to have failed, so the count stays neutral",
|
||||
);
|
||||
assert.equal(await view.settleToStepCount(0), 0, "the rail folds away");
|
||||
assert.equal(
|
||||
view.qa(".animate-pulse").length,
|
||||
0,
|
||||
// Then OPEN it and look. Asserting "nothing pulses" on the folded block would
|
||||
// be vacuous: the fold unmounts every row, so no glyph exists to carry a
|
||||
// pulse class and the assertion would hold against a build where abandoned
|
||||
// steps pulse forever — the exact Codex finding. The reader's complaint is
|
||||
// about what they see when they scroll back and expand, so that is where the
|
||||
// assertion belongs.
|
||||
await view.expand();
|
||||
assert.equal(view.stepCount(), 2, "the expanded rail has rows to inspect");
|
||||
assert.deepEqual(
|
||||
view.glyphStates(),
|
||||
["settled", "settled"],
|
||||
"the abandoned step reads as settled, not running",
|
||||
);
|
||||
assert.deepEqual(
|
||||
view.pulseStates(),
|
||||
[],
|
||||
"nothing pulses when nothing is running",
|
||||
);
|
||||
});
|
||||
@@ -63,7 +76,7 @@ test("the same items still hold the block open while the turn is live", async ()
|
||||
assert.equal(view.summary(), null, "a live block has no folded line");
|
||||
assert.equal(view.stepCount(), 2, "the rail stays open");
|
||||
assert.deepEqual(view.glyphStates(), ["settled", "running"]);
|
||||
assert.equal(view.qa(".animate-pulse").length, 1, "live work pulses");
|
||||
assert.deepEqual(view.pulseStates(), ["running"], "live work pulses");
|
||||
});
|
||||
|
||||
test("an agent live on a later turn does not resurrect an earlier turn's abandoned step", async () => {
|
||||
@@ -79,7 +92,11 @@ test("an agent live on a later turn does not resurrect an earlier turn's abandon
|
||||
"this turn is history even though the agent is busy",
|
||||
);
|
||||
assert.equal(await view.settleToStepCount(0), 0);
|
||||
assert.equal(view.qa(".animate-pulse").length, 0);
|
||||
// Expanded, for the same reason as above: a folded rail has no glyph to pulse,
|
||||
// so the negative has to be taken on rows that exist.
|
||||
await view.expand();
|
||||
assert.deepEqual(view.glyphStates(), ["settled", "settled"]);
|
||||
assert.deepEqual(view.pulseStates(), []);
|
||||
});
|
||||
|
||||
test("a block live when the session ends folds instead of spinning forever", async () => {
|
||||
@@ -105,8 +122,67 @@ test("a block live when the session ends folds instead of spinning forever", asy
|
||||
|
||||
assert.ok(view.summary(), "the block settles when its session goes away");
|
||||
assert.match(view.summary().textContent, /2 steps$/);
|
||||
// Taken HERE, before the fold finishes: the rail is still mounted for the
|
||||
// settle frame, so the glyph that was pulsing a moment ago still exists and
|
||||
// can be asked whether it stopped. Once `settleToStepCount(0)` has run there
|
||||
// are no rows left and the same assertion proves nothing.
|
||||
assert.equal(view.stepCount(), 2, "the rail is still mounted to inspect");
|
||||
assert.deepEqual(view.glyphStates(), ["settled", "settled"]);
|
||||
assert.deepEqual(
|
||||
view.pulseStates(),
|
||||
[],
|
||||
"the pulse stops the moment the session goes away, not when the fold finishes",
|
||||
);
|
||||
assert.equal(await view.settleToStepCount(0), 0);
|
||||
assert.equal(view.qa(".animate-pulse").length, 0);
|
||||
});
|
||||
|
||||
// ── The gap between two turns ─────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* A finished block must stay folded through the gap before the next turn shows
|
||||
* anything.
|
||||
*
|
||||
* The rendered half of the `buildConversationTurnMeta` gap contract (see
|
||||
* `agentSessionConversationMeta.test.mjs`). The meta test proves the hints are
|
||||
* right; this proves the reader sees the consequence, because the symptom was
|
||||
* never a wrong id — it was a settled 6-step block re-opening, dropping to its
|
||||
* last three steps behind a "previous steps" disclosure, and then folding back,
|
||||
* on every single turn.
|
||||
*
|
||||
* Six steps rather than two on purpose: the live window only applies above
|
||||
* three, so a smaller block would hide the loudest part of the regression.
|
||||
*/
|
||||
test("a finished block stays folded while the next turn has started but shown nothing", async () => {
|
||||
const { act } = await import("@testing-library/react");
|
||||
const items = ["a", "b", "c", "d", "e", "f"].map((id) => step(id));
|
||||
|
||||
// Finished: nothing live, so the block is folded to its summary line.
|
||||
const view = await renderBlock(items, {
|
||||
liveTurnId: null,
|
||||
streamingItemId: null,
|
||||
});
|
||||
assert.match(view.summary().textContent, /6 steps$/);
|
||||
assert.equal(await view.settleToStepCount(0), 0, "it starts folded");
|
||||
|
||||
// The next turn starts. It owns liveness (turn-2) and, having emitted nothing
|
||||
// renderable, contributes no streaming item — which is exactly what the fixed
|
||||
// `latestTurnId`/`streamingIdForTail` pair reports for this frame.
|
||||
await act(async () => {
|
||||
view.stream(items, null, "turn-2");
|
||||
});
|
||||
|
||||
assert.ok(
|
||||
view.summary(),
|
||||
"the folded summary line survives the next turn starting",
|
||||
);
|
||||
assert.match(view.summary().textContent, /6 steps$/);
|
||||
assert.equal(view.stepCount(), 0, "the rail does not re-open");
|
||||
assert.equal(
|
||||
view.previousSteps(),
|
||||
null,
|
||||
"and the live window's previous-steps disclosure never appears",
|
||||
);
|
||||
assert.deepEqual(view.pulseStates(), []);
|
||||
});
|
||||
|
||||
test("an orphaned step keeps its own row detail rather than gaining an interrupted marker", async () => {
|
||||
|
||||
@@ -341,12 +341,62 @@ test("a running step pulses and a failed one does not", async () => {
|
||||
{ streamingItemId: "b" },
|
||||
);
|
||||
const [failed, running] = view.qa("[data-step-state]");
|
||||
assert.ok(
|
||||
running.className.includes("animate-pulse"),
|
||||
"work in flight pulses",
|
||||
assert.deepEqual(
|
||||
view.pulseStates(),
|
||||
["running"],
|
||||
"work in flight pulses and a settled failure does not",
|
||||
);
|
||||
assert.ok(
|
||||
!failed.className.includes("animate-pulse"),
|
||||
assert.equal(
|
||||
failed.classList.contains("motion-safe:animate-pulse"),
|
||||
false,
|
||||
"a settled failure is not in flight",
|
||||
);
|
||||
assert.equal(
|
||||
running.classList.contains("motion-safe:animate-pulse"),
|
||||
true,
|
||||
"the running step is the one that pulses",
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* Reduced motion has to reach this pulse through the CLASS, not the hook.
|
||||
*
|
||||
* `useWorkBlockMotionEnabled` skips the fold's height animation, but the running
|
||||
* glyph's pulse is a CSS keyframe animation applied by a Tailwind utility, which
|
||||
* no hook can switch off — so the guard has to be `motion-safe:animate-pulse`.
|
||||
* Compiled with this repo's Tailwind (4.3.0), `animate-pulse` is
|
||||
* `animation: var(--animate-pulse)` with no guard, while
|
||||
* `motion-safe:animate-pulse` wraps exactly that declaration in
|
||||
* `@media (prefers-reduced-motion: no-preference)`. jsdom does not evaluate
|
||||
* media queries, so the assertion is on the class: the bare form would keep
|
||||
* pulsing forever for a reader who asked for no motion, and none of the app's 20
|
||||
* `prefers-reduced-motion: reduce` blocks matches `.animate-pulse` (all are
|
||||
* scoped to `buzz-*`/`motion-*`/`t-skel-*` classes), so nothing else would catch
|
||||
* it.
|
||||
*
|
||||
* The preference is irrelevant to which class is emitted, and that is the point:
|
||||
* asserting under BOTH settings is what proves the guard is in the class rather
|
||||
* than in a conditional that reduced motion could flip.
|
||||
*/
|
||||
test("a running glyph's pulse is guarded so reduced motion silences it", async () => {
|
||||
for (const reduced of [false, true]) {
|
||||
setPrefersReducedMotion(reduced);
|
||||
const view = await renderBlock(
|
||||
[step("a"), step("b", { status: "executing", completedAt: null })],
|
||||
{ streamingItemId: "b" },
|
||||
);
|
||||
const [, running] = view.qa("[data-step-state]");
|
||||
assert.equal(
|
||||
running.classList.contains("motion-safe:animate-pulse"),
|
||||
true,
|
||||
`the pulse is reduced-motion-safe (prefersReducedMotion=${reduced})`,
|
||||
);
|
||||
assert.deepEqual(
|
||||
view.unguardedPulseStates(),
|
||||
[],
|
||||
`no glyph pulses unconditionally (prefersReducedMotion=${reduced})`,
|
||||
);
|
||||
const { cleanup } = await import("@testing-library/react");
|
||||
cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -601,7 +601,18 @@ function WorkBlockRailGlyph({
|
||||
<div
|
||||
className={cn(
|
||||
"relative z-10 mt-0.5 flex size-5 items-center justify-center rounded-full bg-background text-muted-foreground ring-2 ring-background",
|
||||
entry.state === "running" && "animate-pulse",
|
||||
// `motion-safe:`, not bare `animate-pulse`: the guard has to be in the
|
||||
// class because this animation is CSS, not motion. The block's height
|
||||
// animation is skipped through `useWorkBlockMotionEnabled`, but that
|
||||
// hook cannot reach a keyframe animation applied by a utility — and no
|
||||
// reduced-motion rule in the app's CSS matches `.animate-pulse` (every
|
||||
// one of them is scoped to a `buzz-*`/`motion-*` class), so an
|
||||
// unguarded pulse would keep pulsing forever for a reader who asked
|
||||
// for no motion. Tailwind compiles `motion-safe:` to
|
||||
// `@media (prefers-reduced-motion: no-preference)`, which is the only
|
||||
// thing that actually stops it. Matches every other pulse in this
|
||||
// feature (`AgentStatusBadge`, `ManagedAgentRow`).
|
||||
entry.state === "running" && "motion-safe:animate-pulse",
|
||||
)}
|
||||
data-step-state={entry.state}
|
||||
>
|
||||
|
||||
@@ -305,6 +305,31 @@ export async function renderBlock(
|
||||
qa("[data-step-state]").map((node) =>
|
||||
node.getAttribute("data-step-state"),
|
||||
),
|
||||
// Which rail glyphs pulse, reported as their step states, and the same
|
||||
// narrowed to pulses that are NOT reduced-motion-safe.
|
||||
//
|
||||
// Both read `classList.contains`, which matches a whole class token, rather
|
||||
// than a `.animate-pulse` CSS selector or `className.includes` —
|
||||
// `animate-pulse` is a SUBSTRING of `motion-safe:animate-pulse`, so a
|
||||
// substring check cannot tell the guarded class from the bare one and would
|
||||
// report the reduced-motion regression as fixed.
|
||||
//
|
||||
// They return step-state STRINGS rather than the elements: a failing
|
||||
// `deepEqual` over jsdom nodes makes node:test serialize the whole DOM tree
|
||||
// and the runner dies on SIGKILL instead of printing an assertion, which
|
||||
// silently turns "the mutant was caught" into an unreadable crash.
|
||||
pulseStates: () =>
|
||||
qa("[data-step-state]")
|
||||
.filter(
|
||||
(node) =>
|
||||
node.classList.contains("motion-safe:animate-pulse") ||
|
||||
node.classList.contains("animate-pulse"),
|
||||
)
|
||||
.map((node) => node.getAttribute("data-step-state")),
|
||||
unguardedPulseStates: () =>
|
||||
qa("[data-step-state]")
|
||||
.filter((node) => node.classList.contains("animate-pulse"))
|
||||
.map((node) => node.getAttribute("data-step-state")),
|
||||
q,
|
||||
qa,
|
||||
};
|
||||
|
||||
@@ -3,6 +3,7 @@ import test from "node:test";
|
||||
|
||||
import { buildConversationTurnMeta } from "./agentSessionConversationMeta.ts";
|
||||
import { EMPTY_TRANSCRIPT_TURN_META } from "./agentSessionTranscriptContext.ts";
|
||||
import { buildTranscriptDisplayBlocks } from "./agentSessionTranscriptGrouping.ts";
|
||||
import {
|
||||
formatPlanChecklistProgress,
|
||||
parsePlanChecklist,
|
||||
@@ -17,10 +18,114 @@ function item(overrides) {
|
||||
};
|
||||
}
|
||||
|
||||
const RAW_TIMESTAMP = "2026-06-14T19:00:00.000Z";
|
||||
|
||||
/**
|
||||
* Raw transcript items, for the tests that go through the real grouping rather
|
||||
* than asserting on hand-written blocks. `acpSource` is what decides whether an
|
||||
* item is setup lifecycle, a prompt, or work, so these carry it.
|
||||
*/
|
||||
function lifecycleItem(id, acpSource, turnId) {
|
||||
return item({
|
||||
id,
|
||||
type: "lifecycle",
|
||||
renderClass: "status",
|
||||
title: acpSource,
|
||||
text: "",
|
||||
timestamp: RAW_TIMESTAMP,
|
||||
acpSource,
|
||||
turnId,
|
||||
});
|
||||
}
|
||||
|
||||
function promptItem(id, turnId) {
|
||||
return item({
|
||||
id,
|
||||
type: "message",
|
||||
role: "user",
|
||||
renderClass: "message",
|
||||
title: "Buzz event",
|
||||
text: "please look into this",
|
||||
timestamp: RAW_TIMESTAMP,
|
||||
acpSource: "session/prompt:user",
|
||||
turnId,
|
||||
});
|
||||
}
|
||||
|
||||
function toolItem(id, turnId) {
|
||||
return item({
|
||||
id,
|
||||
type: "tool",
|
||||
renderClass: "shell",
|
||||
title: id,
|
||||
toolName: "shell",
|
||||
buzzToolName: null,
|
||||
status: "completed",
|
||||
args: {},
|
||||
result: "ok",
|
||||
isError: false,
|
||||
timestamp: RAW_TIMESTAMP,
|
||||
startedAt: RAW_TIMESTAMP,
|
||||
completedAt: RAW_TIMESTAMP,
|
||||
descriptor: {
|
||||
renderClass: "shell",
|
||||
label: "Ran command",
|
||||
preview: id,
|
||||
source: "shell",
|
||||
groupKey: "shell:command",
|
||||
},
|
||||
turnId,
|
||||
});
|
||||
}
|
||||
|
||||
function systemPromptItem(id) {
|
||||
return item({
|
||||
id,
|
||||
type: "metadata",
|
||||
renderClass: "metadata",
|
||||
title: "System prompt",
|
||||
sections: [],
|
||||
timestamp: RAW_TIMESTAMP,
|
||||
acpSource: "session/new",
|
||||
turnId: null,
|
||||
});
|
||||
}
|
||||
|
||||
function turnBlock(segments) {
|
||||
return { kind: "turn", turnId: "turn-1", segments };
|
||||
}
|
||||
|
||||
/**
|
||||
* The leaf items a block is built from, in wire order — including setup
|
||||
* lifecycle items, which carry a turn id in the real stream even though they
|
||||
* contribute nothing to the streaming tail.
|
||||
*
|
||||
* `buildConversationTurnMeta` reads the item stream as well as the blocks (it
|
||||
* has to: a turn that has only emitted setup rows produces no block at all), so
|
||||
* a test that passed blocks alone would be describing a transcript that cannot
|
||||
* exist. This keeps the two arguments consistent by construction.
|
||||
*/
|
||||
function blockItems(blocks) {
|
||||
return blocks.flatMap((block) => {
|
||||
if (block.kind === "single") return [block.item];
|
||||
if (block.kind !== "turn") return [];
|
||||
return block.segments.flatMap((segment) => {
|
||||
if (segment.kind === "prompt") return [segment.user];
|
||||
if (segment.kind === "setup") return segment.items;
|
||||
if (segment.kind === "summary") return segment.summary.items;
|
||||
return [segment.item];
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
/** `buildConversationTurnMeta` with the item stream derived from the blocks. */
|
||||
function metaFor(blocks, { items, ...options }) {
|
||||
return buildConversationTurnMeta(blocks, {
|
||||
...options,
|
||||
items: items ?? blockItems(blocks),
|
||||
});
|
||||
}
|
||||
|
||||
function itemSegment(id, overrides) {
|
||||
return {
|
||||
kind: "item",
|
||||
@@ -66,7 +171,7 @@ const toolSegment = (id, timestamp) =>
|
||||
test("buildConversationTurnMeta returns the shared empty value for other variants", () => {
|
||||
for (const variant of ["default", "compactPreview"]) {
|
||||
assert.equal(
|
||||
buildConversationTurnMeta([turnBlock([])], {
|
||||
metaFor([turnBlock([])], {
|
||||
isTurnLive: true,
|
||||
variant,
|
||||
}),
|
||||
@@ -80,7 +185,7 @@ test("buildConversationTurnMeta reports nothing streaming when the turn is idle"
|
||||
// The hint exists to tell the work block it is still working. A finished turn
|
||||
// has no tail, and reporting one would pin the block open forever.
|
||||
assert.equal(
|
||||
buildConversationTurnMeta(
|
||||
metaFor(
|
||||
[
|
||||
turnBlock([
|
||||
thoughtSegment("thought:1", "2026-06-14T19:00:02.000Z"),
|
||||
@@ -99,7 +204,7 @@ test("buildConversationTurnMeta names the trailing item of a live turn", () => {
|
||||
];
|
||||
|
||||
assert.equal(
|
||||
buildConversationTurnMeta(blocks, {
|
||||
metaFor(blocks, {
|
||||
isTurnLive: true,
|
||||
variant: "conversation",
|
||||
}).streamingItemId,
|
||||
@@ -111,7 +216,7 @@ test("buildConversationTurnMeta names the trailing item of a live turn", () => {
|
||||
test("buildConversationTurnMeta skips setup segments when finding the tail", () => {
|
||||
// Setup renders as a quiet divider, not as work, so it must never be reported
|
||||
// as the streaming item — a lifecycle row would hold the block open.
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[
|
||||
turnBlock([
|
||||
thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z"),
|
||||
@@ -137,7 +242,7 @@ test("buildConversationTurnMeta skips setup segments when finding the tail", ()
|
||||
});
|
||||
|
||||
test("buildConversationTurnMeta counts a mid-turn steer prompt as the tail", () => {
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[
|
||||
turnBlock([
|
||||
thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z"),
|
||||
@@ -153,7 +258,7 @@ test("buildConversationTurnMeta counts a mid-turn steer prompt as the tail", ()
|
||||
test("buildConversationTurnMeta expands a summary segment to reach its last tool", () => {
|
||||
// A summary segment holds several leaf items; the tail is the last of them,
|
||||
// not the summary itself, which has no item id the work block could match.
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[
|
||||
turnBlock([
|
||||
{
|
||||
@@ -180,7 +285,7 @@ test("buildConversationTurnMeta expands a summary segment to reach its last tool
|
||||
});
|
||||
|
||||
test("buildConversationTurnMeta reads a trailing single block directly", () => {
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[
|
||||
turnBlock([thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z")]),
|
||||
{
|
||||
@@ -209,7 +314,7 @@ test("buildConversationTurnMeta reads a trailing single block directly", () => {
|
||||
* ownership is knowledge only the list has, so it is published here.
|
||||
*/
|
||||
test("buildConversationTurnMeta names the live turn so the block can gate running steps", () => {
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[turnBlock([thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z")])],
|
||||
{ isTurnLive: true, variant: "conversation" },
|
||||
);
|
||||
@@ -220,7 +325,7 @@ test("buildConversationTurnMeta names the live turn so the block can gate runnin
|
||||
test("buildConversationTurnMeta reports no live turn when the turn is idle", () => {
|
||||
// This is the case the orphaned-step bug lived in: history reopened, nothing
|
||||
// live, but an item still claiming `executing`.
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[turnBlock([thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z")])],
|
||||
{ isTurnLive: false, variant: "conversation" },
|
||||
);
|
||||
@@ -238,7 +343,7 @@ test("buildConversationTurnMeta names the live turn even when a lifecycle row tr
|
||||
// to, so "the last block" is not always the live turn — but the last turn is.
|
||||
// Reading the last block's kind alone would report no live turn and gate every
|
||||
// running step off, which is the mirror image of the bug being fixed.
|
||||
const meta = buildConversationTurnMeta(
|
||||
const meta = metaFor(
|
||||
[
|
||||
turnBlock([thoughtSegment("thought:1", "2026-06-14T19:00:01.000Z")]),
|
||||
{
|
||||
@@ -260,7 +365,12 @@ test("buildConversationTurnMeta names the live turn even when a lifecycle row tr
|
||||
});
|
||||
|
||||
test("buildConversationTurnMeta reports no live turn when there is no turn at all", () => {
|
||||
const meta = buildConversationTurnMeta(
|
||||
// `turnId: null` explicitly: a session-started row arrives before any turn
|
||||
// exists, so it belongs to nobody. The shared `item()` helper defaults to
|
||||
// `turn-1`, which would have made this fixture describe a transcript that
|
||||
// does have a turn — and the assertion would then be checking that a turn
|
||||
// with no BLOCK reports no live turn, which is the opposite of the rule.
|
||||
const meta = metaFor(
|
||||
[
|
||||
{
|
||||
kind: "single",
|
||||
@@ -271,6 +381,7 @@ test("buildConversationTurnMeta reports no live turn when there is no turn at al
|
||||
title: "Session started",
|
||||
text: "",
|
||||
timestamp: "2026-06-14T19:00:00.000Z",
|
||||
turnId: null,
|
||||
}),
|
||||
},
|
||||
],
|
||||
@@ -280,6 +391,117 @@ test("buildConversationTurnMeta reports no live turn when there is no turn at al
|
||||
assert.equal(meta.liveTurnId, null);
|
||||
});
|
||||
|
||||
// ---- buildConversationTurnMeta: the gap between two turns ----
|
||||
|
||||
/**
|
||||
* The next turn owns liveness the moment it appears, even before it has anything
|
||||
* to show.
|
||||
*
|
||||
* A turn that has only emitted setup lifecycle rows (`turn_started`,
|
||||
* `session_resolved`) classifies to zero segments and therefore produces NO
|
||||
* block — `agentSessionTranscriptGrouping` only pushes a turn block
|
||||
* `if (segments.length > 0)`. So "the newest turn with a block" is the turn that
|
||||
* ALREADY ENDED for the whole gap between `turn_started` and the new turn's
|
||||
* first prompt or thought, and the ended turn's own trailing item then gets
|
||||
* reported as the streaming item. Its finished work block reads as live again:
|
||||
* the folded summary is replaced by the open rail, a 6-step block drops to its
|
||||
* last three with a previous-steps disclosure, and then it all folds back.
|
||||
*
|
||||
* That gap is real observer-stream latency, and `turn_started` fires on every
|
||||
* turn (`activeAgentTurnsStore`), so this was not an edge case — it was every
|
||||
* turn, for as long as the agent took to emit its first renderable item.
|
||||
*
|
||||
* Built from a raw item stream through the real grouping rather than from
|
||||
* hand-written blocks, because the whole bug is which turns DO and DO NOT
|
||||
* produce a block: hand-built blocks would assume the answer away.
|
||||
*/
|
||||
test("buildConversationTurnMeta hands liveness to a next turn that has no block yet", () => {
|
||||
const stream = (...extra) => [
|
||||
lifecycleItem("life:start:1", "turn_started", "turn-1"),
|
||||
promptItem("prompt:1", "turn-1"),
|
||||
toolItem("tool:1", "turn-1"),
|
||||
// turn-1's last act is a completed tool: the agent answered by posting
|
||||
// through buzz-cli, so no assistant message trails its work.
|
||||
toolItem("tool:2", "turn-1"),
|
||||
...extra,
|
||||
];
|
||||
|
||||
const frames = [
|
||||
{
|
||||
label: "turn-2 has only started",
|
||||
items: stream(lifecycleItem("life:start:2", "turn_started", "turn-2")),
|
||||
},
|
||||
{
|
||||
label: "the new session's system prompt has landed",
|
||||
items: stream(
|
||||
lifecycleItem("life:start:2", "turn_started", "turn-2"),
|
||||
systemPromptItem("meta:sysprompt"),
|
||||
),
|
||||
},
|
||||
{
|
||||
label: "the session has resolved",
|
||||
items: stream(
|
||||
lifecycleItem("life:start:2", "turn_started", "turn-2"),
|
||||
systemPromptItem("meta:sysprompt"),
|
||||
lifecycleItem("life:resolved:2", "session_resolved", "turn-2"),
|
||||
),
|
||||
},
|
||||
];
|
||||
|
||||
for (const frame of frames) {
|
||||
const meta = buildConversationTurnMeta(
|
||||
buildTranscriptDisplayBlocks(frame.items),
|
||||
{ isTurnLive: true, items: frame.items, variant: "conversation" },
|
||||
);
|
||||
assert.equal(
|
||||
meta.liveTurnId,
|
||||
"turn-2",
|
||||
`the newest turn owns liveness once ${frame.label}`,
|
||||
);
|
||||
assert.equal(
|
||||
meta.streamingItemId,
|
||||
null,
|
||||
`turn-1's tail must not read as streaming once ${frame.label}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Fourth frame: turn-2 finally has something renderable, so it gets a block
|
||||
// and its own prompt becomes the tail. Included so the sequence ends in the
|
||||
// steady state rather than stopping at the gap.
|
||||
const renderable = stream(
|
||||
lifecycleItem("life:start:2", "turn_started", "turn-2"),
|
||||
systemPromptItem("meta:sysprompt"),
|
||||
lifecycleItem("life:resolved:2", "session_resolved", "turn-2"),
|
||||
promptItem("prompt:2", "turn-2"),
|
||||
);
|
||||
const meta = buildConversationTurnMeta(
|
||||
buildTranscriptDisplayBlocks(renderable),
|
||||
{ isTurnLive: true, items: renderable, variant: "conversation" },
|
||||
);
|
||||
assert.equal(meta.liveTurnId, "turn-2");
|
||||
assert.equal(meta.streamingItemId, "prompt:2");
|
||||
});
|
||||
|
||||
test("buildConversationTurnMeta still names the live turn's own trailing item", () => {
|
||||
// The other half of the ownership check: when the newest turn IS the turn that
|
||||
// owns the trailing item, that item is the streaming one. Without this the
|
||||
// gate would be a blanket "never report a tail" and every live block would
|
||||
// fold while the reader watched it arrive.
|
||||
const items = [
|
||||
lifecycleItem("life:start:1", "turn_started", "turn-1"),
|
||||
promptItem("prompt:1", "turn-1"),
|
||||
toolItem("tool:1", "turn-1"),
|
||||
];
|
||||
const meta = buildConversationTurnMeta(buildTranscriptDisplayBlocks(items), {
|
||||
isTurnLive: true,
|
||||
items,
|
||||
variant: "conversation",
|
||||
});
|
||||
|
||||
assert.equal(meta.liveTurnId, "turn-1");
|
||||
assert.equal(meta.streamingItemId, "tool:1");
|
||||
});
|
||||
|
||||
// ---- parsePlanChecklist ----
|
||||
|
||||
test("parsePlanChecklist reads the checkbox markdown the transcript builds", () => {
|
||||
|
||||
@@ -48,29 +48,91 @@ function turnSegmentItems(segment: TranscriptTurnSegment): TranscriptItem[] {
|
||||
*/
|
||||
export function buildConversationTurnMeta(
|
||||
displayBlocks: TranscriptDisplayBlock[],
|
||||
options: { isTurnLive: boolean; variant: AgentSessionTranscriptVariant },
|
||||
options: {
|
||||
isTurnLive: boolean;
|
||||
items: TranscriptItem[];
|
||||
variant: AgentSessionTranscriptVariant;
|
||||
},
|
||||
): AgentSessionTranscriptTurnMeta {
|
||||
if (options.variant !== "conversation" || !options.isTurnLive) {
|
||||
return EMPTY_TRANSCRIPT_TURN_META;
|
||||
}
|
||||
|
||||
const lastBlock = displayBlocks[displayBlocks.length - 1];
|
||||
// The live turn is the last turn in the transcript: work arrives in order, so
|
||||
// a turn with anything after it has already been superseded. Read from the
|
||||
// blocks rather than from the active-turn store because the store's turn ids
|
||||
// and the transcript's are populated by different paths, and a mismatch would
|
||||
// silently gate every step off.
|
||||
const liveTurnId = lastTurnId(displayBlocks);
|
||||
// The live turn is the newest turn in the ITEM stream, not the newest turn
|
||||
// that produced a block. See `latestTurnId` for why the distinction is
|
||||
// load-bearing. Read from the transcript rather than from the active-turn
|
||||
// store because the store's turn ids and the transcript's are populated by
|
||||
// different paths, and a mismatch would silently gate every step off.
|
||||
const liveTurnId = latestTurnId(options.items, displayBlocks);
|
||||
|
||||
if (lastBlock?.kind === "single") {
|
||||
return { liveTurnId, streamingItemId: lastBlock.item.id };
|
||||
return {
|
||||
liveTurnId,
|
||||
streamingItemId: streamingIdForTail(lastBlock.item, liveTurnId),
|
||||
};
|
||||
}
|
||||
if (lastBlock?.kind !== "turn") {
|
||||
return { liveTurnId, streamingItemId: null };
|
||||
}
|
||||
|
||||
const items = lastBlock.segments.flatMap(turnSegmentItems);
|
||||
return { liveTurnId, streamingItemId: items[items.length - 1]?.id ?? null };
|
||||
const tail = items[items.length - 1];
|
||||
return {
|
||||
liveTurnId,
|
||||
streamingItemId: tail ? streamingIdForTail(tail, liveTurnId) : null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* The trailing item counts as streaming only if the LIVE turn owns it.
|
||||
*
|
||||
* Without this check the newest block's tail is reported as streaming no matter
|
||||
* whose turn it belongs to, so a finished turn's last step would hold that
|
||||
* turn's work block open while a *different* turn is the live one. That is the
|
||||
* same class of bug as an abandoned `executing` step: presenting settled history
|
||||
* as work in flight.
|
||||
*/
|
||||
function streamingIdForTail(
|
||||
tail: TranscriptItem,
|
||||
liveTurnId: string | null,
|
||||
): string | null {
|
||||
// Compared rather than tested for truthiness, for the same reason as
|
||||
// `toolEntryState`: an item with no turn id is owned by nobody, and
|
||||
// `null === null` must not read as ownership.
|
||||
if (liveTurnId === null || tail.turnId !== liveTurnId) return null;
|
||||
return tail.id;
|
||||
}
|
||||
|
||||
/**
|
||||
* The id of the newest turn the transcript has seen — including a turn that has
|
||||
* arrived but has nothing renderable in it yet.
|
||||
*
|
||||
* Read from the items rather than the blocks because a turn that has only
|
||||
* emitted setup lifecycle rows (`turn_started`, `session_resolved`) classifies
|
||||
* to zero segments and so produces NO block at all
|
||||
* (`agentSessionTranscriptGrouping`: `if (segments.length > 0)`). Walking the
|
||||
* blocks backwards for the last `turn` therefore skipped straight past the new
|
||||
* turn and returned the turn that had already ended — which then made its own
|
||||
* trailing item the streaming item, and a settled block visibly re-opened,
|
||||
* dropped to its last three steps and re-expanded for the whole gap between
|
||||
* `turn_started` and the new turn's first prompt or thought. That gap is real
|
||||
* observer-stream latency and happens on every turn, so this was not an edge
|
||||
* case.
|
||||
*
|
||||
* Items are in wire order, so the last one carrying a turn id names the newest
|
||||
* turn. Falls back to the last turn block for an item stream that carries no
|
||||
* turn ids at all.
|
||||
*/
|
||||
function latestTurnId(
|
||||
items: TranscriptItem[],
|
||||
displayBlocks: TranscriptDisplayBlock[],
|
||||
): string | null {
|
||||
for (let index = items.length - 1; index >= 0; index -= 1) {
|
||||
const turnId = items[index]?.turnId;
|
||||
if (turnId) return turnId;
|
||||
}
|
||||
return lastTurnBlockId(displayBlocks);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -80,7 +142,9 @@ export function buildConversationTurnMeta(
|
||||
* turn it belongs to and arrives as a `single` block, so the last block is not
|
||||
* always the live turn — but the last *turn* is.
|
||||
*/
|
||||
function lastTurnId(displayBlocks: TranscriptDisplayBlock[]): string | null {
|
||||
function lastTurnBlockId(
|
||||
displayBlocks: TranscriptDisplayBlock[],
|
||||
): string | null {
|
||||
for (let index = displayBlocks.length - 1; index >= 0; index -= 1) {
|
||||
const block = displayBlocks[index];
|
||||
if (block.kind === "turn") return block.turnId;
|
||||
|
||||
Reference in New Issue
Block a user