From 5cb4f294373d3165baa31e356db8ccd35c7a226d Mon Sep 17 00:00:00 2001 From: Adam Dalloul <47503782+Adam-Dalloul@users.noreply.github.com> Date: Wed, 9 Sep 2026 19:18:11 -0700 Subject: [PATCH] fix(chat): keep a reply the live stream is not showing Two timeline rules hide a persisted assistant turn while a reply streams: the `liveOwnsActiveTurn` tail strip, which drops everything after the last persisted user turn for a delegation-child viewer, and the `in_flight_user_turn_id` suppression, which drops the partial assistant turn OpenCode and Gemini persist mid-stream. Both are sound only because the live stream is showing that same reply, and both asked whether a live message OBJECT existed rather than whether it renders anything. Those two differ, routinely. `status_changed` to `prompting` installs a fresh `content: []` live message at the start of every turn and the dispatch mirror pushes it into the runtime store, which acp-connections-context.test.tsx already pins ("fires with isLive=true and a fresh non-null liveMessage when a turn starts"). The mirror never writes a null back over it, so the same object stays in hand through any stretch of a turn that produces nothing the streaming build renders. In that state the persisted reply was hidden with nothing put in its place: a blank agent turn. The sub-agent dialog reaches it on an ordinary path. `LiveTranscriptView` marks the session `liveOwnsActiveTurn` on mount, bridges whatever live message the child's connection holds, and fetches with `preserveLive: true`, so when the child starts its next turn the strip runs against an empty live message and the round already on screen disappears until the first chunk arrives. The child CLI writes its JSONL asynchronously, so a refetch inside that window keeps the same shape. `computeTimeline` now builds the streaming tail first and passes the verdict down: a live message suppresses a persisted reply only when it produced an assistant turn. Deriving that from the same build is what keeps the two from disagreeing again. A live message carrying only a mid-turn steering message counts as showing no reply, since that turn is the user's, not a rendering of the reply it would otherwise hide. --- .../conversation-runtime-context.test.tsx | 39 +++- src/stores/conversation-runtime-store.ts | 62 ++++-- src/stores/runtime-empty-live-message.test.ts | 206 ++++++++++++++++++ 3 files changed, 287 insertions(+), 20 deletions(-) create mode 100644 src/stores/runtime-empty-live-message.test.ts diff --git a/src/contexts/conversation-runtime-context.test.tsx b/src/contexts/conversation-runtime-context.test.tsx index 2c635ba5c5..602c9e324d 100644 --- a/src/contexts/conversation-runtime-context.test.tsx +++ b/src/contexts/conversation-runtime-context.test.tsx @@ -709,6 +709,14 @@ describe("ConversationRuntimeProvider delegation kickoff projection", () => { mockGetFolderConversation.mockImplementation(() => new Promise(() => {})) }) + /** The live message the strip stands in for: one that IS showing the reply. */ + const streamingReply: LiveMessage = { + id: "lm-streaming", + role: "assistant", + content: [{ type: "text", text: "working on it" }], + startedAt: 0, + } + it("synthesizes the kickoff user turn (and strips the persisted reply) while the transcript has no user turn yet", async () => { // DB lags: only a partial assistant turn is persisted, no user turn. mockGetFolderConversation.mockResolvedValueOnce( @@ -721,7 +729,7 @@ describe("ConversationRuntimeProvider delegation kickoff projection", () => { api().setLiveOwnsActiveTurn(99, true, "do the thing") }) act(() => { - api().setLiveMessage(99, LIVE_MSG, true) + api().setLiveMessage(99, streamingReply, true) }) await act(async () => { api().refetchDetail(99, { preserveLive: true }) @@ -744,6 +752,35 @@ describe("ConversationRuntimeProvider delegation kickoff projection", () => { ).toBe(false) }) + it("keeps the persisted reply while the live message is showing nothing", async () => { + // The child's next turn has begun: `status_changed → prompting` put a fresh + // `content: []` live message on the connection and the viewer bridged it, + // but no chunk has arrived. Stripping the reply then leaves the dialog + // showing a prompt with nothing under it. + mockGetFolderConversation.mockResolvedValueOnce( + detailWithTurns([userTurn("u1"), assistantTurn("a1")]) + ) + renderProvider() + const api = () => runtimeHolder.current! + + act(() => { + api().setLiveOwnsActiveTurn(99, true, "do the thing") + }) + act(() => { + api().setLiveMessage(99, LIVE_MSG, true) + }) + await act(async () => { + api().refetchDetail(99, { preserveLive: true }) + await Promise.resolve() + }) + + expect( + api() + .getTimelineTurns(99) + .map((t) => t.turn.id) + ).toEqual(["u1", "a1"]) + }) + it("uses the real persisted user turn instead of synthesizing once it has landed", async () => { mockGetFolderConversation.mockResolvedValueOnce( detailWithTurns([userTurn("u1"), assistantTurn("a1")]) diff --git a/src/stores/conversation-runtime-store.ts b/src/stores/conversation-runtime-store.ts index 64fa185355..22eb517231 100644 --- a/src/stores/conversation-runtime-store.ts +++ b/src/stores/conversation-runtime-store.ts @@ -2780,7 +2780,7 @@ interface TimelinePrefixDeps { optimisticTurns: MessageTurn[] liveOwnsActiveTurn: boolean delegationKickoffText: string | null - hasLiveMessage: boolean + liveShowsReply: boolean liveStartedAt: number | null } interface TimelinePrefixEntry { @@ -3029,7 +3029,7 @@ function timelinePrefixDepsEqual( a.optimisticTurns === b.optimisticTurns && a.liveOwnsActiveTurn === b.liveOwnsActiveTurn && a.delegationKickoffText === b.delegationKickoffText && - a.hasLiveMessage === b.hasLiveMessage && + a.liveShowsReply === b.liveShowsReply && a.liveStartedAt === b.liveStartedAt ) } @@ -3078,9 +3078,22 @@ function collectInFlightPersistedToolCalls( return out } +/** + * @param liveShowsReply whether the live message this session holds actually + * produced an assistant turn — see [`computeTimeline`], which derives it from + * the same build the streaming tail is made of. Both suppressions below hide a + * persisted assistant turn *because the live stream is showing that reply*, so + * they must key off what the live message RENDERS, never off the existence of a + * live message object. Those two differ: a live message with nothing renderable + * in it is an ordinary state (`STATUS_CHANGED` → `prompting` installs + * `content: []` at the start of every turn, and the runtime mirror never writes + * a null back over it), and keying off the object hid a reply with nothing put + * in its place — a blank agent turn. + */ function computeTimelinePrefix( session: ConversationRuntimeSession, - conversationId: number + conversationId: number, + liveShowsReply: boolean ): TimelinePrefixEntry { const detail = session.detail // Everything Phases 1–3 read, snapshotted for the `===` validity check. @@ -3094,7 +3107,7 @@ function computeTimelinePrefix( optimisticTurns: session.optimisticTurns, liveOwnsActiveTurn: session.liveOwnsActiveTurn, delegationKickoffText: session.delegationKickoffText, - hasLiveMessage: session.liveMessage !== null, + liveShowsReply, liveStartedAt: session.liveMessage?.startedAt ?? null, } if (detail) { @@ -3121,7 +3134,7 @@ function computeTimelinePrefix( const rawPersistedTurns = session.detail?.turns ?? [] const hasLiveOrLocalReply = session.liveOwnsActiveTurn && - (session.liveMessage !== null || session.localTurns.length > 0) + (liveShowsReply || session.localTurns.length > 0) let stripFrom = -1 if (hasLiveOrLocalReply) { let lastUserIdx = -1 @@ -3153,13 +3166,12 @@ function computeTimelinePrefix( // into `detail` it sits beside the live reply (a separate assistant turn // under a `live-…` id), and `mergeConsecutiveAssistantTurns` concatenates // the two — so the already-persisted head (e.g. the first reasoning block) - // renders twice. Hide that persisted partial, but ONLY while `liveMessage` - // is in hand: the live stream carries the full reply (the attach snapshot is - // built atomically and includes it), so this only ever hides from render - // what the live stream is concurrently showing — never dropping a reply we - // can't re-show. The moment the turn ends, `liveMessage` clears and the - // persisted copy (now complete) renders normally; the brief promote→refetch - // grace window can show a transient visible duplicate, never a hidden turn. + // renders twice. Hide that persisted partial, but ONLY while the live message + // is actually SHOWING a reply (`liveShowsReply`): that is what makes this a + // choice between two renderings of one reply rather than a deletion. The + // moment the turn ends, `liveMessage` clears and the persisted copy (now + // complete) renders normally; the brief promote→refetch grace window can show + // a transient visible duplicate, never a hidden turn. // // The in-flight prompt is identified authoritatively by the backend, which // reports the id of the persisted user turn it stamped as the in-flight one @@ -3171,9 +3183,7 @@ function computeTimelinePrefix( // id, so an earlier completed round's reply is never mistaken for a partial. const inFlightPromptId = session.detail?.in_flight_user_turn_id ?? null const inFlightPromptIdx = - !hasLiveOrLocalReply && - session.liveMessage !== null && - inFlightPromptId !== null + !hasLiveOrLocalReply && liveShowsReply && inFlightPromptId !== null ? persistedTurns.findIndex( (t) => t.role === "user" && t.id === inFlightPromptId ) @@ -3415,14 +3425,28 @@ function computeTimeline( const cached = timelineCache.get(session) if (cached) return cached - // Phases 1–3 (already deduped), reused across streaming batches. - const { prefix, prefixKeys } = computeTimelinePrefix(session, conversationId) - - // Phase 4: Streaming turns (live agent response, split into rounds) + // Phase 4 first: Phases 1–3 hide the persisted copy of the reply this build + // is showing, so they need its verdict, and deriving that from the same build + // is what keeps the two from disagreeing. A live message can hold nothing + // renderable — `content: []` from the turn's own `prompting` transition, or + // only blocks this build drops — and a check for the message OBJECT then hid + // a persisted reply that nothing replaced. const streamingMessage = session.liveMessage const built = streamingMessage ? buildStreamingTurnsFromLiveMessage(conversationId, streamingMessage) : null + // A `user` turn here is a message the user sent mid-turn (native steering), + // not a rendering of the reply — a live message that produced only those is + // showing no reply and must suppress nothing. + const liveShowsReply = + built?.turns.some((turn) => turn.role === "assistant") ?? false + + // Phases 1–3 (already deduped), reused across streaming batches. + const { prefix, prefixKeys } = computeTimelinePrefix( + session, + conversationId, + liveShowsReply + ) let deduped: ConversationTimelineTurn[] if (!built || built.turns.length === 0) { diff --git a/src/stores/runtime-empty-live-message.test.ts b/src/stores/runtime-empty-live-message.test.ts new file mode 100644 index 0000000000..175b6a26c2 --- /dev/null +++ b/src/stores/runtime-empty-live-message.test.ts @@ -0,0 +1,206 @@ +/** + * Two timeline rules hide a persisted assistant turn while a reply streams: the + * `liveOwnsActiveTurn` tail strip (delegation-child dialog) and the + * `in_flight_user_turn_id` partial suppression (cross-client viewer). Both are + * only sound because the live stream is showing that same reply — so both have + * to key off what the live message RENDERS, not off a live message existing. + * + * Those differ, and routinely. `status_changed → prompting` installs a fresh + * `content: []` live message at the start of every turn and mirrors it into + * this store (see "fires with isLive=true and a fresh non-null liveMessage when + * a turn starts" in acp-connections-context.test.tsx); the mirror never writes a + * null back over it, so the same object stays in hand for any part of a turn + * that produces nothing this build renders. Keyed on the object, the persisted + * reply was hidden with nothing put in its place: a blank agent turn. + */ + +import { afterEach, describe, expect, it } from "vitest" + +import type { LiveMessage } from "@/contexts/acp-connections-context" +import type { DbConversationDetail, MessageTurn, TurnRole } from "@/lib/types" +import { + getTimelineTurns, + resetConversationRuntimeStore, + useConversationRuntimeStore, +} from "@/stores/conversation-runtime-store" + +const CID = 77 +const TS = "2026-09-06T00:00:00.000Z" + +function turn(id: string, role: TurnRole): MessageTurn { + return { id, role, blocks: [{ type: "text", text: id }], timestamp: TS } +} + +/** What the turn's own `prompting` transition installs, before any content. */ +const promptingLiveMessage: LiveMessage = { + id: "m1", + role: "assistant", + content: [], + startedAt: Date.parse(TS), +} + +/** A block Phase 2 drops, so this message renders exactly as much as `[]`. */ +const emptyTextLiveMessage: LiveMessage = { + ...promptingLiveMessage, + content: [{ type: "text", text: "" }], +} + +const replyLiveMessage: LiveMessage = { + ...promptingLiveMessage, + content: [{ type: "text", text: "streaming…" }], +} + +/** A message the user sent mid-turn, with no reply to it yet. */ +const steeringOnlyLiveMessage: LiveMessage = { + ...promptingLiveMessage, + content: [ + { + type: "steering", + id: "note-1", + text: "also check the tests", + createdAt: TS, + }, + ], +} + +function seed( + turns: MessageTurn[], + overrides: { + liveMessage?: LiveMessage | null + liveOwnsActiveTurn?: boolean + localTurns?: MessageTurn[] + inFlightUserTurnId?: string | null + } +) { + const detail: DbConversationDetail = { + summary: { + id: CID, + folder_id: 1, + title: "t", + title_locked: false, + agent_type: "claude_code", + status: "in_progress", + kind: "regular", + model: null, + git_branch: null, + external_id: null, + message_count: turns.length, + child_count: 0, + created_at: TS, + updated_at: TS, + pinned_at: null, + }, + turns, + in_flight_user_turn_id: overrides.inFlightUserTurnId ?? null, + } + const next = new Map(useConversationRuntimeStore.getState().byConversationId) + next.set(CID, { + conversationId: CID, + externalId: null, + dbConversationId: null, + detail, + detailLoading: false, + detailError: null, + acpLoadError: null, + localTurns: overrides.localTurns ?? [], + backgroundTurns: [], + pendingBackgroundSettlements: [], + optimisticTurns: [], + liveMessage: overrides.liveMessage ?? null, + syncState: "idle" as const, + activeTurnToken: null, + lastTurnOwned: false, + liveOwnsActiveTurn: overrides.liveOwnsActiveTurn ?? false, + delegationKickoffText: null, + sessionStats: null, + historyAssistantBaseline: null, + batchBoundaryIndex: null, + batchBoundaryPrefixHash: null, + loadingOlderTurns: false, + olderTurnsPrependEpoch: 0, + pendingCleanup: false, + }) + useConversationRuntimeStore.setState({ byConversationId: next }) +} + +const timelineIds = () => getTimelineTurns(CID).map((t) => t.turn.id) + +afterEach(() => { + resetConversationRuntimeStore() +}) + +describe("persisted-tail strip vs. a live message that renders nothing", () => { + it("keeps the child's reply while the new turn has produced nothing yet", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveOwnsActiveTurn: true, + liveMessage: promptingLiveMessage, + }) + expect(timelineIds()).toEqual(["u1", "a1"]) + }) + + it("keeps the child's reply when the live message holds only an empty block", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveOwnsActiveTurn: true, + liveMessage: emptyTextLiveMessage, + }) + expect(timelineIds()).toEqual(["u1", "a1"]) + }) + + it("still strips the persisted copy once the live message shows the reply", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveOwnsActiveTurn: true, + liveMessage: replyLiveMessage, + }) + expect(timelineIds()).toEqual(["u1", `live-${CID}-m1`]) + }) + + it("still strips for a promoted reply, which renders on its own", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveOwnsActiveTurn: true, + liveMessage: promptingLiveMessage, + localTurns: [turn("promoted", "assistant")], + }) + expect(timelineIds()).toEqual(["u1", "promoted"]) + }) +}) + +describe("in-flight partial suppression vs. a live message that renders nothing", () => { + it("keeps the persisted partial while the turn has produced nothing yet", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveMessage: promptingLiveMessage, + inFlightUserTurnId: "u1", + }) + expect(timelineIds()).toEqual(["u1", "a1"]) + }) + + it("keeps every persisted reply of the round, not just the newest", () => { + seed( + [ + turn("u1", "user"), + turn("a1", "assistant"), + turn("a2", "assistant"), + turn("a3", "assistant"), + ], + { liveMessage: emptyTextLiveMessage, inFlightUserTurnId: "u1" } + ) + expect(timelineIds()).toEqual(["u1", "a1", "a2", "a3"]) + }) + + it("still hides the persisted partial once the live message shows the reply", () => { + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveMessage: replyLiveMessage, + inFlightUserTurnId: "u1", + }) + expect(timelineIds()).toEqual(["u1", `live-${CID}-m1`]) + }) + + it("keeps the persisted reply when the live message carries only a steer", () => { + // A mid-turn message is the user's, not a rendering of the reply, so it + // cannot stand in for the persisted copy it would otherwise hide. + seed([turn("u1", "user"), turn("a1", "assistant")], { + liveMessage: steeringOnlyLiveMessage, + inFlightUserTurnId: "u1", + }) + expect(timelineIds()).toEqual(["u1", "a1", `live-${CID}-m1`]) + }) +})