mirror of
https://github.com/wu736139669/hapi.git
synced 2026-08-05 06:24:37 +00:00
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 <cursoragent@cursor.com> * 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 <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -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'
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -325,6 +325,9 @@ export function SessionChat(props: {
|
||||
const normalized: NormalizedMessage[] = []
|
||||
const seen = new Set<string>()
|
||||
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) {
|
||||
|
||||
@@ -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<ToolGr
|
||||
}
|
||||
}
|
||||
|
||||
describe('assignThreadMessageIds', () => {
|
||||
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<VisibleChatBlock, BlockWithThreadMessageId>()
|
||||
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
|
||||
|
||||
@@ -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<VisibleChatBlock, BlockWithThreadMessageId>
|
||||
): BlockWithThreadMessageId[] {
|
||||
const seen = new Map<string, number>()
|
||||
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<VisibleChatBlock, BlockWithThreadMessageId>()
|
||||
)
|
||||
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<VisibleChatBlock>({
|
||||
const convertedMessages = useExternalMessageConverter<BlockWithThreadMessageId>({
|
||||
callback: convertBlock,
|
||||
messages: props.blocks as VisibleChatBlock[],
|
||||
messages: blocksWithThreadIds,
|
||||
isRunning,
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user