From 0127d5c9534578cae0bf13bbceddc4d8c3855f6e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:19:16 -0700 Subject: [PATCH] fix(native-chat): stop the send/reply reflow in structured chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two seams made a structured turn visibly jump. Frame-stepping a 54fps capture of one send measured both: - The host renders the user bubble from the submission WAL row, which lands while dispatch is still `pending`, but the optimistic outbox echo is only retired on `accepted` — so the same bubble drew twice for the whole provider round trip (~74ms) and then collapsed. Desktop drops the echo once its WAL row exists; mobile adopts the canonical row so Retry / edit-queued stay reachable on it. - The trailing "…" row keyed its visibility on the PTY path's synthetic streaming id, which the structured path never mints, so it stayed pinned below the reply until `isWorking` cleared (~110ms later) — a second reflow. The rule moves to a pure module that scans back to the turn boundary, so a system row mid-turn cannot flicker the dots back on. --- ...mobile-structured-session-timeline.test.ts | 17 +++++ .../mobile-structured-session-timeline.ts | 37 +++++++--- .../native-chat/NativeChatMessageList.tsx | 5 +- .../native-chat-typing-indicator.test.ts | 72 +++++++++++++++++++ .../native-chat-typing-indicator.ts | 32 +++++++++ ...d-agent-session-message-projection.test.ts | 33 +++++++++ ...ctured-agent-session-message-projection.ts | 26 ++++--- 7 files changed, 201 insertions(+), 21 deletions(-) create mode 100644 src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-typing-indicator.ts diff --git a/mobile/src/session/mobile-structured-session-timeline.test.ts b/mobile/src/session/mobile-structured-session-timeline.test.ts index 8e75f385e5c..2f15cf264d8 100644 --- a/mobile/src/session/mobile-structured-session-timeline.test.ts +++ b/mobile/src/session/mobile-structured-session-timeline.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import type { AgentJournalRenderItem } from '../../../src/shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' import { buildMobileStructuredTimeline, activeMobileStructuredTurnId, @@ -74,6 +75,22 @@ describe('mobile structured session timeline', () => { }) }) + it('adopts the WAL row instead of drawing the send twice while it dispatches', () => { + const wal: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey(OUTBOX.clientMessageId), + revision: 0, + sequence: 1, + observedAt: 3, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'look' }] } + } + + const rows = buildMobileStructuredTimeline([wal], [OUTBOX]) + + expect(rows).toHaveLength(1) + // The entry rides the canonical row, so Retry / edit-queued stay reachable. + expect(rows[0]).toMatchObject({ key: wal.itemId, outbox: { state: 'unconfirmed' } }) + }) + it('restores host paths and local previews when a queued send is edited', () => { expect(restoreMobileStructuredAttachments(OUTBOX)).toEqual([ { diff --git a/mobile/src/session/mobile-structured-session-timeline.ts b/mobile/src/session/mobile-structured-session-timeline.ts index 77f2f62bdba..dafe07e8c6a 100644 --- a/mobile/src/session/mobile-structured-session-timeline.ts +++ b/mobile/src/session/mobile-structured-session-timeline.ts @@ -1,4 +1,5 @@ import type { AgentJournalRenderItem } from '../../../src/shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../src/shared/agent-session-journal-item-key' import type { NativeChatMessage } from '../../../src/shared/native-chat-types' import type { MobileStructuredPromptItem } from './MobileStructuredPromptCard' import type { PendingNativeChatImage } from './mobile-native-chat-image-attachment' @@ -45,23 +46,41 @@ export function buildMobileStructuredTimeline( items: readonly AgentJournalRenderItem[], outbox: readonly MobileStructuredOutboxEntry[] ): MobileStructuredTimelineRow[] { + // Why: the host renders its own bubble off the submission WAL row, which lands + // while the dispatch is still `pending` — before reconciliation retires the echo + // on `accepted`. Appending both drew the message twice for the whole provider + // round trip, so adopt the canonical row and keep the entry on it: retry and + // edit-queued still need the entry while delivery is unconfirmed. + const adopted = new Map( + outbox.map((entry) => [agentJournalSubmissionKey(entry.clientMessageId), entry]) + ) + const seen = new Set() const canonical = items.flatMap((item): MobileStructuredTimelineRow[] => { if (isPendingPrompt(item)) { return [{ kind: 'prompt', key: item.itemId, item }] } const message = projectStructuredItemToNativeChat(item) - return message ? [{ kind: 'message', key: message.id, message }] : [] + if (!message) { + return [] + } + const entry = adopted.get(item.itemId) + if (entry) { + seen.add(entry.clientMessageId) + } + return [{ kind: 'message', key: message.id, message, ...(entry ? { outbox: entry } : {}) }] }) return [ ...canonical, - ...outbox.map( - (entry): MobileStructuredTimelineRow => ({ - kind: 'message', - key: entry.clientMessageId, - message: outboxMessage(entry), - outbox: entry - }) - ) + ...outbox + .filter((entry) => !seen.has(entry.clientMessageId)) + .map( + (entry): MobileStructuredTimelineRow => ({ + kind: 'message', + key: entry.clientMessageId, + message: outboxMessage(entry), + outbox: entry + }) + ) ] } diff --git a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx index f7a5d89bb09..3a174691835 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageList.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageList.tsx @@ -19,7 +19,7 @@ import { isNearBottom, shouldShowJumpToLatest, type ScrollGeometry } from './nat import { isNativeChatPastedImagePath } from './native-chat-image-paste' import { NativeChatToolRun } from './NativeChatToolRun' import { NativeChatCopyButton } from './NativeChatCopyButton' -import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming' +import { shouldShowNativeChatTypingIndicator } from './native-chat-typing-indicator' function geometryOf(el: HTMLElement): ScrollGeometry { return { scrollTop: el.scrollTop, scrollHeight: el.scrollHeight, clientHeight: el.clientHeight } @@ -311,8 +311,7 @@ export function NativeChatMessageList({ () => foldToolMessages(orderNativeChatMessages(stripNoiseMessages(session.messages))), [session.messages] ) - const showTypingIndicator = - isWorking && !messages.some((message) => message.id === NATIVE_CHAT_STREAMING_ID) + const showTypingIndicator = shouldShowNativeChatTypingIndicator({ messages, isWorking }) // When an older page prepends, the scroll content grows above the viewport. // Capture the pre-render scroll height so the layout effect can restore the diff --git a/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts b/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts new file mode 100644 index 00000000000..1dad39ef89a --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-typing-indicator.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from 'vitest' +import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming' +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { shouldShowNativeChatTypingIndicator } from './native-chat-typing-indicator' + +function message(id: string, role: NativeChatMessage['role'], text = id): NativeChatMessage { + return { id, role, blocks: [{ type: 'text', text }], timestamp: null, source: 'transcript' } +} + +describe('shouldShowNativeChatTypingIndicator', () => { + it('stays hidden when the session is idle', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [message('u1', 'user')], + isWorking: false + }) + ).toBe(false) + }) + + it('shows once a send lands and no assistant row exists yet', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [message('a0', 'assistant'), message('u1', 'user')], + isWorking: true + }) + ).toBe(true) + }) + + it('hides as soon as the structured reply row arrives, before working clears', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [message('u1', 'user'), message('orca-item', 'assistant')], + isWorking: true + }) + ).toBe(false) + }) + + it('hides behind the PTY streaming bubble', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [message('u1', 'user'), message(NATIVE_CHAT_STREAMING_ID, 'assistant')], + isWorking: true + }) + ).toBe(false) + }) + + it('does not flicker back on when a system row interleaves mid-turn', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [ + message('u1', 'user'), + message('a1', 'assistant'), + message('s1', 'system', 'Ran /status') + ], + isWorking: true + }) + ).toBe(false) + }) + + it('shows again for the next send even though an earlier turn replied', () => { + expect( + shouldShowNativeChatTypingIndicator({ + messages: [message('u1', 'user'), message('a1', 'assistant'), message('u2', 'user')], + isWorking: true + }) + ).toBe(true) + }) + + it('shows on a session whose transcript is still empty', () => { + expect(shouldShowNativeChatTypingIndicator({ messages: [], isWorking: true })).toBe(true) + }) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts b/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts new file mode 100644 index 00000000000..44883851666 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-typing-indicator.ts @@ -0,0 +1,32 @@ +// When the trailing "…" row is allowed to render. The rule is pure so the two +// transports can't drift: the PTY path grows a synthetic streaming bubble while +// the structured path appends real journal items, but both mean the same thing — +// the turn's own assistant row is on screen, so a second placeholder below it is +// just an extra row that reflows the list when it later disappears. + +import type { NativeChatMessage } from '../../../../shared/native-chat-types' +import { NATIVE_CHAT_STREAMING_ID } from '../../../../shared/native-chat-streaming' + +export function shouldShowNativeChatTypingIndicator(args: { + messages: readonly NativeChatMessage[] + isWorking: boolean +}): boolean { + if (!args.isWorking) { + return false + } + const { messages } = args + // Scan back only to the turn boundary: an assistant row from an EARLIER turn + // must not suppress the indicator for the send the user just made. + for (let index = messages.length - 1; index >= 0; index -= 1) { + const message = messages[index] + if (!message || message.role === 'user') { + return true + } + // Status/system rows interleave mid-turn; they neither suppress nor unsuppress, + // otherwise the dots would flicker back on between assistant chunks. + if (message.role === 'assistant' || message.id === NATIVE_CHAT_STREAMING_ID) { + return false + } + } + return true +} diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts index e7d30cb5e0b..392ae4662e6 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.test.ts @@ -3,6 +3,7 @@ import type { AgentJournalRenderItem, AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' import { createStructuredAgentSessionOutboxEntry } from '../../../../shared/structured-agent-session-outbox' import { projectStructuredAgentSessionMessages } from './structured-agent-session-message-projection' @@ -52,6 +53,38 @@ describe('structured agent session message projection', () => { ) }) + it('renders one bubble while the submission is still dispatching', () => { + const outbox = [ + createStructuredAgentSessionOutboxEntry({ + clientMessageId: 'client-pending', + sessionId: 'session-1', + text: 'Ok thanks', + attachments: [], + queuedAt: 1 + }) + ] + // The host's WAL row is on screen while the provider round trip is in flight. + const walItem: AgentJournalRenderItem = { + itemId: agentJournalSubmissionKey('client-pending'), + revision: 0, + sequence: 1, + observedAt: 1, + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'Ok thanks' }] } + } + const pending: AgentJournalSubmission = { + ...submission(0), + clientMessageId: 'client-pending', + dispatchState: 'pending', + providerItemId: null, + resolvedAt: null + } + + const messages = projectStructuredAgentSessionMessages([walItem], outbox, [pending]) + + expect(messages.filter((message) => message.role === 'user')).toHaveLength(1) + expect(messages.map((message) => message.id)).toEqual([walItem.itemId]) + }) + it('keeps an optimistic send until its acceptance arrives', () => { const outbox = [ createStructuredAgentSessionOutboxEntry({ diff --git a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts index 434885a715c..68534966689 100644 --- a/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts +++ b/src/renderer/src/components/native-chat/structured-agent-session-message-projection.ts @@ -2,6 +2,7 @@ import type { AgentJournalRenderItem, AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' +import { agentJournalSubmissionKey } from '../../../../shared/agent-session-journal-item-key' import type { NativeChatMessage } from '../../../../shared/native-chat-types' import { reconcileStructuredAgentSessionOutbox, @@ -15,16 +16,23 @@ export function projectStructuredAgentSessionMessages( submissions: readonly AgentJournalSubmission[] ): NativeChatMessage[] { const optimistic = reconcileStructuredAgentSessionOutbox(outbox, submissions) + // Why: the host renders its own bubble off the submission WAL row, which lands + // while the dispatch is still `pending`. Reconciliation only retires the echo on + // `accepted`, so keying visibility on that alone double-rendered the bubble for + // the whole provider round trip. The entry itself stays for retry/unconfirmed. + const journalled = new Set(items.map((item) => item.itemId)) return [ ...projectStructuredItemsToNativeChat(items), - ...optimistic.map( - (entry): NativeChatMessage => ({ - id: entry.clientMessageId, - role: 'user', - source: 'transcript', - timestamp: entry.queuedAt, - blocks: entry.body.blocks - }) - ) + ...optimistic + .filter((entry) => !journalled.has(agentJournalSubmissionKey(entry.clientMessageId))) + .map( + (entry): NativeChatMessage => ({ + id: entry.clientMessageId, + role: 'user', + source: 'transcript', + timestamp: entry.queuedAt, + blocks: entry.body.blocks + }) + ) ] }