From c1eccc0de2885c08c3e11a7847b66c00b8bd59a1 Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Wed, 27 May 2026 00:13:16 +0100 Subject: [PATCH] fix(web): guarantee unique assistant-ui thread message IDs (#706) * fix(web): guarantee unique assistant-ui thread message IDs Suffix duplicate kind:id pairs before ExternalStore sync; skip duplicate hub rows in SessionChat normalization. Align outline scroll targets with the new user-text thread id shape. Closes #704. Co-authored-by: Cursor * fix(web): reuse thread-id wrappers for useExternalMessageConverter cache WeakMap stable BlockWithThreadMessageId objects keyed on reconciled block refs so streaming appends do not invalidate assistant-ui converter caches (PR #706 review). Co-authored-by: Cursor --------- Co-authored-by: Cursor --- web/src/chat/outline.test.ts | 10 +-- web/src/chat/outline.ts | 4 +- .../AssistantChat/HappyThread.test.tsx | 16 ++-- web/src/components/SessionChat.tsx | 3 + web/src/lib/assistant-runtime.test.ts | 34 ++++++- web/src/lib/assistant-runtime.ts | 90 ++++++++++++++----- 6 files changed, 119 insertions(+), 38 deletions(-) diff --git a/web/src/chat/outline.test.ts b/web/src/chat/outline.test.ts index b801b13f..152b4d92 100644 --- a/web/src/chat/outline.test.ts +++ b/web/src/chat/outline.test.ts @@ -33,8 +33,8 @@ describe('conversation outline', () => { userBlock('m1', 'Implement the outline panel', 1000), ])).toEqual([ { - id: 'outline:user:m1', - targetMessageId: 'user:m1', + id: 'outline:user-text:m1', + targetMessageId: 'user-text:m1', kind: 'user', label: 'Implement the outline panel', createdAt: 1000 @@ -67,7 +67,7 @@ describe('conversation outline', () => { ]) expect(items.map((item) => item.id)).toEqual([ - 'outline:user:sent' + 'outline:user-text:sent' ]) }) @@ -79,8 +79,8 @@ describe('conversation outline', () => { ]) expect(items.map((item) => item.id)).toEqual([ - 'outline:user:first', - 'outline:user:second' + 'outline:user-text:first', + 'outline:user-text:second' ]) }) }) diff --git a/web/src/chat/outline.ts b/web/src/chat/outline.ts index 24134abd..f325bfaf 100644 --- a/web/src/chat/outline.ts +++ b/web/src/chat/outline.ts @@ -25,8 +25,8 @@ export function truncateOutlineLabel(value: string, maxLength = MAX_OUTLINE_LABE function userBlockToOutlineItem(block: UserTextBlock): ConversationOutlineItem { const label = truncateOutlineLabel(block.text) || 'Empty message' return { - id: `outline:user:${block.id}`, - targetMessageId: `user:${block.id}`, + id: `outline:user-text:${block.id}`, + targetMessageId: `user-text:${block.id}`, kind: 'user', label, createdAt: block.createdAt diff --git a/web/src/components/AssistantChat/HappyThread.test.tsx b/web/src/components/AssistantChat/HappyThread.test.tsx index f3ea814c..c81ec3db 100644 --- a/web/src/components/AssistantChat/HappyThread.test.tsx +++ b/web/src/components/AssistantChat/HappyThread.test.tsx @@ -14,15 +14,15 @@ import type { ConversationOutlineItem } from '@/chat/outline' const outlineItems: ConversationOutlineItem[] = [ { - id: 'outline:user:m1', - targetMessageId: 'user:m1', + id: 'outline:user-text:m1', + targetMessageId: 'user-text:m1', kind: 'user', label: 'Implement the panel', createdAt: 1000 }, { - id: 'outline:user:m2', - targetMessageId: 'user:m2', + id: 'outline:user-text:m2', + targetMessageId: 'user-text:m2', kind: 'user', label: 'Second user prompt', createdAt: 2000 @@ -195,14 +195,14 @@ describe('outline target loading', () => { }) const findTarget = vi.fn((anchorId: string) => { - if (anchorId !== 'hapi-message-user:target') { + if (anchorId !== 'hapi-message-user-text:target') { return null } return loadCount >= 2 ? document.createElement('div') : null }) const target = await locateOutlineTargetMessage({ - targetMessageId: 'user:target', + targetMessageId: 'user-text:target', findTarget, hasMoreMessages: () => loadCount < 2, loadOlderPreservingScroll @@ -210,14 +210,14 @@ describe('outline target loading', () => { expect(target).toBeInstanceOf(HTMLElement) expect(loadOlderPreservingScroll).toHaveBeenCalledTimes(2) - expect(findTarget).toHaveBeenCalledWith('hapi-message-user:target') + expect(findTarget).toHaveBeenCalledWith('hapi-message-user-text:target') }) it('stops when history is exhausted before the target is loaded', async () => { const loadOlderPreservingScroll = vi.fn(async () => false) const target = await locateOutlineTargetMessage({ - targetMessageId: 'user:missing', + targetMessageId: 'user-text:missing', findTarget: () => null, hasMoreMessages: () => true, loadOlderPreservingScroll diff --git a/web/src/components/SessionChat.tsx b/web/src/components/SessionChat.tsx index 2eb7498d..586a3495 100644 --- a/web/src/components/SessionChat.tsx +++ b/web/src/components/SessionChat.tsx @@ -325,6 +325,9 @@ export function SessionChat(props: { const normalized: NormalizedMessage[] = [] const seen = new Set() for (const message of visibleMessages) { + if (seen.has(message.id)) { + continue + } seen.add(message.id) const cached = cache.get(message.id) if (cached && cached.source === message) { diff --git a/web/src/lib/assistant-runtime.test.ts b/web/src/lib/assistant-runtime.test.ts index c5c01978..ef97f0bc 100644 --- a/web/src/lib/assistant-runtime.test.ts +++ b/web/src/lib/assistant-runtime.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { aggregateResponseGroups } from './assistant-runtime' +import { + type BlockWithThreadMessageId, + aggregateResponseGroups, + assignThreadMessageIds, + assignThreadMessageIdsWithStableWrappers +} from './assistant-runtime' import type { AgentEventBlock, AgentTextBlock, CliOutputBlock, ToolCallBlock, UserTextBlock } from '@/chat/types' import type { ToolGroupBlock, VisibleChatBlock } from '@/chat/toolGroups' @@ -99,6 +104,33 @@ function toolGroup(id: string, tools: ToolCallBlock[], overrides: Partial { + it('suffixes duplicate kind+id pairs so assistant-ui never sees repeated thread ids', () => { + const blocks: VisibleChatBlock[] = [ + agentText('dup'), + userText('u1'), + agentText('dup') + ] + + const assigned = assignThreadMessageIds(blocks) + expect(assigned.map((entry) => entry.threadMessageId)).toEqual([ + 'agent-text:dup', + 'user-text:u1', + 'agent-text:dup~1' + ]) + }) + + it('reuses wrapper objects from a WeakMap cache when block ref and thread id are unchanged', () => { + const block = agentText('a') + const cache = new WeakMap() + const first = assignThreadMessageIdsWithStableWrappers([block], cache) + const second = assignThreadMessageIdsWithStableWrappers([block, userText('u')], cache) + expect(second[0]).toBe(first[0]) + expect(second[0].threadMessageId).toBe('agent-text:a') + expect(second[1].threadMessageId).toBe('user-text:u') + }) +}) + describe('aggregateResponseGroups', () => { it('1. sums usage and dedups model across distinct localIds in a single response group', () => { // user (no aggregate) → agent-text L1 → tool-call L1 → tool-call L2 → agent-text L3 diff --git a/web/src/lib/assistant-runtime.ts b/web/src/lib/assistant-runtime.ts index 9c3d542e..7769ead3 100644 --- a/web/src/lib/assistant-runtime.ts +++ b/web/src/lib/assistant-runtime.ts @@ -1,4 +1,4 @@ -import { useCallback, useMemo } from 'react' +import { useCallback, useMemo, useRef } from 'react' import type React from 'react' import type { AppendMessage, AttachmentAdapter, ThreadMessageLike } from '@assistant-ui/react' import { useExternalMessageConverter, useExternalStoreRuntime } from '@assistant-ui/react' @@ -276,12 +276,53 @@ export function aggregateResponseGroups( return aggregates } -function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { +export type BlockWithThreadMessageId = { + block: VisibleChatBlock + threadMessageId: string +} + +/** + * Stable, unique IDs for assistant-ui's linear MessageRepository. + * Uses `${kind}:${block.id}`; suffixes `~1`, `~2`, … when the same kind+id + * appears more than once (should be rare — indicates duplicate hub rows or + * a reducer bug, but must not crash the thread). + * + * Reuses `{ block, threadMessageId }` objects from `wrapperCache` when the + * reconciled `block` reference and computed id match, so + * `useExternalMessageConverter`'s WeakMap caches stay warm across streaming + * appends (see PR review). + */ +export function assignThreadMessageIdsWithStableWrappers( + blocks: readonly VisibleChatBlock[], + wrapperCache: WeakMap +): BlockWithThreadMessageId[] { + const seen = new Map() + return blocks.map((block) => { + const base = `${block.kind}:${block.id}` + const occurrence = seen.get(base) ?? 0 + seen.set(base, occurrence + 1) + const threadMessageId = occurrence === 0 ? base : `${base}~${occurrence}` + const cached = wrapperCache.get(block) + if (cached?.threadMessageId === threadMessageId) { + return cached + } + const next: BlockWithThreadMessageId = { block, threadMessageId } + wrapperCache.set(block, next) + return next + }) +} + +export function assignThreadMessageIds( + blocks: readonly VisibleChatBlock[] +): BlockWithThreadMessageId[] { + return assignThreadMessageIdsWithStableWrappers(blocks, new WeakMap()) +} + +function toThreadMessageLike(block: VisibleChatBlock, threadMessageId: string): ThreadMessageLike { if (block.kind === 'user-text') { - const messageId = `user:${block.id}` return { role: 'user', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'text', text: block.text }], metadata: { @@ -298,10 +339,9 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } if (block.kind === 'agent-text') { - const messageId = `assistant:${block.id}` return { role: 'assistant', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'text', text: block.text }], metadata: { @@ -319,7 +359,7 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { if (block.kind === 'generated-image') { return { role: 'assistant', - id: `generated-image:${block.id}`, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'tool-call', @@ -339,10 +379,9 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } if (block.kind === 'agent-reasoning') { - const messageId = `assistant:${block.id}` return { role: 'assistant', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'reasoning', text: block.text }], metadata: { @@ -358,10 +397,9 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } if (block.kind === 'codex-review') { - const messageId = `review:${block.id}` return { role: 'assistant', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'text', text: formatCodexReviewText(block.review) }], metadata: { @@ -378,10 +416,9 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } if (block.kind === 'agent-event') { - const messageId = `event:${block.id}` return { role: 'system', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'text', text: renderEventLabel(block.event) }], metadata: { @@ -396,10 +433,9 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } if (block.kind === 'cli-output') { - const messageId = `cli:${block.id}` return { role: block.source === 'user' ? 'user' : 'assistant', - id: messageId, + id: threadMessageId, createdAt: new Date(block.createdAt), content: [{ type: 'text', text: block.text }], metadata: { @@ -419,7 +455,7 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { const groupBlock: ToolGroupBlock = block return { role: 'assistant', - id: `tool:${groupBlock.id}`, + id: threadMessageId, createdAt: new Date(groupBlock.createdAt), content: [{ type: 'tool-call', @@ -439,12 +475,11 @@ function toThreadMessageLike(block: VisibleChatBlock): ThreadMessageLike { } const toolBlock: ToolCallBlock = block - const messageId = `tool:${toolBlock.id}` const inputText = safeStringify(toolBlock.tool.input) return { role: 'assistant', - id: messageId, + id: threadMessageId, createdAt: new Date(toolBlock.createdAt), content: [{ type: 'tool-call', @@ -541,14 +576,25 @@ export function useHappyRuntime(props: { // The library's `joinExternalMessages` only preserves // `metadata.custom` from the first block of a joined chunk, so this // is the surface that survives the join. + const threadIdWrapperCacheRef = useRef( + new WeakMap() + ) + const blocksWithThreadIds = useMemo( + () => assignThreadMessageIdsWithStableWrappers( + props.blocks, + threadIdWrapperCacheRef.current + ), + [props.blocks] + ) + const aggregates = useMemo( () => aggregateResponseGroups(props.blocks), [props.blocks] ) const convertBlock = useCallback( - (block: VisibleChatBlock): ThreadMessageLike => { - const message = toThreadMessageLike(block) + ({ block, threadMessageId }: BlockWithThreadMessageId): ThreadMessageLike => { + const message = toThreadMessageLike(block, threadMessageId) const aggregate = aggregates.get(block.id) if (!aggregate) return message const existing = message.metadata?.custom as HappyChatMessageMetadata | undefined @@ -572,9 +618,9 @@ export function useHappyRuntime(props: { // Use cached message converter for performance optimization // This prevents re-converting all messages on every render - const convertedMessages = useExternalMessageConverter({ + const convertedMessages = useExternalMessageConverter({ callback: convertBlock, - messages: props.blocks as VisibleChatBlock[], + messages: blocksWithThreadIds, isRunning, })