Skip to content

fix(web): guarantee unique assistant-ui thread message IDs#706

Merged
tiann merged 2 commits into
tiann:mainfrom
heavygee:fix/web-unique-assistant-ui-thread-ids
May 26, 2026
Merged

fix(web): guarantee unique assistant-ui thread message IDs#706
tiann merged 2 commits into
tiann:mainfrom
heavygee:fix/web-unique-assistant-ui-thread-ids

Conversation

@heavygee
Copy link
Copy Markdown
Contributor

Summary

  • Assign stable per-block thread ids (${kind}:${id}) with ~1, ~2, … suffixes when the same kind+id repeats, so @assistant-ui/react never receives duplicate ids in the linear external-store array (fix(web): SessionChat crashes when assistant-ui receives duplicate thread message IDs #704).
  • Skip duplicate hub message.id rows when building normalizedMessages in SessionChat (defense in depth).
  • Update conversation outline targetMessageId / outline item ids to user-text:… so outline scroll-to-message matches the new user thread message ids (regression that would otherwise break jump-to-message).

Test plan

  • cd web && bun run test src/lib/assistant-runtime.test.ts src/chat/outline.test.ts src/components/AssistantChat/HappyThread.test.tsx
  • bun typecheck (repo root)

Made with Cursor

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 tiann#704.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Preserve converter cache keys for unchanged blocks — wrapping every VisibleChatBlock in a new { block, threadMessageId } object means useExternalMessageConverter can no longer hit its WeakMap caches when props.blocks changes for a streaming update. The previous call used the stable reconciled block objects directly, so unchanged history was not re-converted; this line now passes fresh wrapper objects for the whole list and turns each append/update into a full visible-history conversion. Evidence web/src/lib/assistant-runtime.ts:599.
    Suggested fix:
    const threadIdCacheRef = useRef(new WeakMap<VisibleChatBlock, BlockWithThreadMessageId>())
    const blocksWithThreadIds = useMemo(() => {
        const seen = new Map<string, number>()
        return props.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 = threadIdCacheRef.current.get(block)
            if (cached?.threadMessageId === threadMessageId) return cached
            const next = { block, threadMessageId }
            threadIdCacheRef.current.set(block, next)
            return next
        })
    }, [props.blocks])

Questions

  • None.

Summary

  • Review mode: initial
  • One performance regression found in the new assistant-ui ID wrapping path. Residual risk: the new duplicate-id behavior needs a regression test that proves unchanged blocks retain converter cache identity across an append/update.

Testing

  • Not run (automation)

HAPI Bot

Comment thread web/src/lib/assistant-runtime.ts
WeakMap stable BlockWithThreadMessageId objects keyed on reconciled block
refs so streaming appends do not invalidate assistant-ui converter caches
(PR tiann#706 review).

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • No issues found with confidence >= 80% in the modified lines.

Questions

  • None.

Summary

  • Review mode: follow-up after new commits
  • The follow-up addresses the prior bot finding by reusing wrapper objects for unchanged VisibleChatBlock references while keeping assistant-ui message IDs unique. Residual risk: I did not run the web test suite in this automation pass; coverage added in the PR targets the ID suffixing, wrapper reuse, and outline anchor rename paths.

Testing

  • Not run (automation)

HAPI Bot

@tiann tiann merged commit c1eccc0 into tiann:main May 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants