mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 08:03:37 +00:00
fix(native-chat): stop the send/reply reflow in structured chat
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.
This commit is contained in:
@@ -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([
|
||||
{
|
||||
|
||||
@@ -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<string>()
|
||||
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
|
||||
})
|
||||
)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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
|
||||
}
|
||||
+33
@@ -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({
|
||||
|
||||
+17
-9
@@ -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
|
||||
})
|
||||
)
|
||||
]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user