chore: upgrade @assistant-ui to 0.14 and replace all custom streaming-rendering code with built-in APIs - #51653
chore: upgrade @assistant-ui to 0.14 and replace all custom streaming-rendering code with built-in APIs#51653okisdev wants to merge 9 commits into
Conversation
Duplicate of #51542 — same forward-upgrade mechanism (@assistant-ui/react 0.12.28→0.14.23 with the appendOptimisticMessage migration in incremental-external-store-runtime.ts) at the same site. #51542 is the earlier open PR for this approach; this one additionally bumps react-streamdown 0.1→0.3 and adds a test. Both are the opposite approach to merged #46707 (which pinned store=0.2.13). Flagging the cluster so a maintainer can pick one. |
…0.1 to 0.3 bumps @assistant-ui/react from ^0.12.28 to ^0.14.23 and @assistant-ui/react-streamdown from ^0.1.11 to ^0.3.4. this crosses two minor bumps on each package and unlocks the built-in defer, smooth, and tail-bounded remend primitives for PR 2. breaking change from core 0.2.x: MessageRepository.appendOptimisticMessage was removed (assistant-ui#4162). inline the three steps it did (generateId + fromThreadMessageLike + addOrUpdateMessage) in incremental-external-store-runtime.ts, and set metadata.isOptimistic so the new off-branch eviction logic cleans up the placeholder correctly. fromThreadMessageLike and generateId graduated to the public API in 0.14.22 (assistant-ui#4414), so they now import from @assistant-ui/react instead of @assistant-ui/core/internal. ExportedMessageRepository in the test file moves to the public import for the same reason. the remaining internal imports (AssistantRuntimeImpl, BaseAssistantRuntimeCore, ExternalStoreThreadListRuntimeCore, ExternalStoreThreadRuntimeCore, hasUpcomingMessage) are runtime construction internals with no public equivalent and stay on @assistant-ui/core/internal. the @assistant-ui/store npm override is removed: all transitive ranges now resolve to 0.2.18 without it. verified: tsc --noEmit passes, vitest shows zero new failures (15 pre-existing, 792 passing, identical to baseline before the upgrade).
…h props
delete SmoothStreamingText, DeferStreamingText, and useSmoothReveal
(~174 lines) from markdown-text.tsx. the built-in defer and smooth
props on StreamdownTextPrimitive now handle the same work:
- defer: routes streaming text through useDeferredValue so markdown
re-parsing runs at lower priority (typing/scrolling stay responsive)
- smooth: typewriter-style reveal via useSmooth with SmoothOptions
{ drainMs: 500, maxCharsPerFrame: 30, minCommitMs: 33 }, matching
the old useSmoothReveal constants exactly
MarkdownTextContent (reasoning text) gets both defer and smooth.
MarkdownText (assistant text) gets defer only, matching the previous
behavior where text messages had no typewriter effect.
the internal pipeline order changes from smooth → defer → preprocess
to preprocess → smooth → defer (the built-in primitive runs preprocess
first). this is functionally equivalent: the tail-bounded remend repair
runs once on the full text instead of per revealed prefix, and the
smooth reveal operates on already-repaired markdown. end result is
identical.
verified: tsc 0 errors, eslint clean, vitest 0 new failures (15
pre-existing, 792 passing), manual verification of 6 streaming
scenarios (defer, smooth reveal, typing-while-streaming, code blocks,
math, long text performance).
…Remend delete lib/remend-tail.ts (108 lines) and lib/remend-tail.test.ts (105 lines). the tailBoundedRemend export from @assistant-ui/react-streamdown 0.3.4 is algorithmically identical — same findRemendWindowStart boundary scan, same fence/math tracking, same slice-and-repair strategy. the only differences are improvements: the built-in handles \r (CR) in line endings for Windows compatibility, and accepts an optional RemendOptions parameter passed through to remend. the import in markdown-text.tsx moves from @/lib/remend-tail to @assistant-ui/react-streamdown. the call site (preprocessWithTailRepair) is unchanged. verified: tsc 0 errors, eslint clean, vitest 0 new failures (15 pre-existing, 786 passing — 6 fewer than before because the deleted remend-tail.test.ts had 6 cases), manual verification of incomplete markdown repair during streaming.
…zeMathDelimiters and escapeCurrencyDollars delete the custom rewriteLatexBracketDelimiters and escapeCurrencyDollars implementations from markdown-preprocess.ts (~40 lines). the built-in exports from @assistant-ui/react-streamdown 0.3.4 are strict improvements: - normalizeMathDelimiters combines rewriteLatexBracketDelimiters (now handles double backslashes and trims body whitespace) with rewriteCustomMathTags (handles [/math]...[/math] and [/inline]...[/inline] tags that some models emit — new capability HA didn't have before) - escapeCurrencyDollars excludes $ as a preceding character, so display math $$5 is no longer incorrectly escaped (bugfix) the call site in preprocessMarkdown changes from rewriteLatexBracketDelimiters(escapeCurrencyDollars(part)) to normalizeMathDelimiters(escapeCurrencyDollars(part)). verified: tsc 0 errors, eslint clean, all 16 preprocessMarkdown tests pass (including currency dollar escaping), vitest 0 new failures, manual verification of currency amounts, LaTeX bracket delimiters, display math, and dollar signs inside code blocks.
b747bf1 to
cab0f69
Compare
OutThisLife
left a comment
There was a problem hiding this comment.
Thanks for this — the direction is great and the net −400 LOC of custom streaming code is exactly the kind of "delete our fork of upstream" cleanup we want. We pulled the branch, rebased it onto current main (it was 939 commits behind; markdown-text.tsx auto-merged cleanly and kept main's newer embeds/alert/RichCodeBlock work), regenerated the lockfile, and verified tsc + the 16 markdown-text tests pass. One heads-up: the PR body says eslint clean, but on a fresh install there was a perfectionist/sort-named-imports error on the new @assistant-ui/react import in incremental-external-store-runtime.ts — we folded a one-line fix into the first commit during the rebase, so it's handled, but worth knowing your local lint may have been stale.
We did a line-by-line comparison of each deleted custom impl against the actual built-in source now in node_modules. Summary: tailBoundedRemend is effectively 1:1 (same constants/scan; built-in just adds CR handling for CRLF + an unused options arg; underlying remend@1.3.0 matches), and the optimistic-message inlining looks faithful. But two of the swaps are not behavior-preserving the way the PR body implies ("end result is identical" / "strict improvements"), so a few questions inline before we merge. None are blockers — mostly want confirmation + a note in the PR body.
General questions:
- The core jump is 0.12 → 0.14 (two minors). Beyond the documented
appendOptimisticMessagebreak, did you audit 0.13/0.14 upstream changelogs for behavior changes in branch handling / message repository / external-store sync that aren't visible in this diff? Anything we should specifically regression-test? - Can you list the exact manual scenarios behind "manual verification: all pass" (models/prompts), so we can reproduce on our side? We're testing via
hguiagainst this branch now.
| // prose segments so code blocks stay untouched. | ||
| const transformed = normalizeVisibleProse( | ||
| stripPreviewTargets(rewriteLatexBracketDelimiters(escapeCurrencyDollars(part))) | ||
| stripPreviewTargets(normalizeMathDelimiters(escapeCurrencyDollars(part))) |
There was a problem hiding this comment.
This swap is a behavior-changing superset, not 1:1 — worth calling out explicitly in the PR body so nobody assumes byte-identical output. Comparing the deleted custom helpers vs the built-ins now resolved (@assistant-ui/react-streamdown@0.3.5):
- body
.trim(): built-in does$${body.trim()}$; custom did$${body}$. So\( x \)(internal padding) now →$x$(renders) where before it produced$ x $(remark-math rejects a$followed by space, so it rendered broken). Net positive, but it is a render change for any model that padded bracket math. \\{1,2}: built-in also matches double-backslash\\(...\\); custom matched single only. New inputs now render as math.rewriteCustomMathTags([/math],[/inline]): brand-new transform with no custom equivalent.- currency regex: built-in
(^|[^\\$])((?:\\\\)*)\$(?=\d)excludes a preceding$, so$$5display math is no longer corrupted into$\$5(the custom(^|[^\\])\$(?=\d)did corrupt it). Bugfix.
Questions:
- Were all four changes intentional, and do you have fixtures covering them (esp.
$$5and the[/math]tags)? The 16 preprocess tests pass but I didn't see cases that would distinguish old vs new here. - Any concern about content that previously rendered a certain way now changing — e.g. prose literally containing
[/math]or\\(that wasn't meant as math? Low risk, but you've widened what gets rewritten.
There was a problem hiding this comment.
agreed on the headline: it's a behavior-changing superset, not 1:1, and i'll say so in the PR body. all four are intentional. three of the four are exactly right: the double-backslash matching, rewriteCustomMathTags, and the currency bugfix where the old (^|[^\\])\$(?=\d) escaped the second $ of $$5x=10$$ into $\$5x=10$$.
one correction on the trim: $ x $ doesn't render broken on the old path. micromark-extension-math (remark-math@6, what resolves here) strips a single leading or trailing space like a code span, so $ x $ and $x$ both parse to inlineMath "x" and render identically. verified directly against the resolved parser. so body.trim() is cosmetic, a cleaner intermediate string, not a fix for a broken render. the one behavioral delta the trim does introduce is the degenerate whitespace-only body: \( \) used to become $ $ (empty, no math node) and is now $$, which can pair with a later $$ and swallow prose. not realistic model output, but it's the one case worth knowing.
on Q1 (fixtures): correct, there were none that distinguish old from new. the 16 preprocessMarkdown tests covered fences, autolinks, citations, and url-in-prose, but no math at all. added four in 94756bd that lock the new behavior on the cases the old helpers got wrong: $$5x = 10$$ stays intact (not escaped as currency), \\(x^2\\) rewrites to $x^2$, [/math]...[/math] and [/inline]...[/inline] rewrite to dollar delimiters, and $5 and $10 is escaped.
on Q2 (widened rewrite): real but bounded. [/math]/[/inline] only rewrite in pairs, so a single literal token in prose is left verbatim; the false-positive case is prose that emits the token twice with content between, which is rare. \(...\) rewriting runs only on prose segments (the fence split excludes code), so \( in code stays intact, and in prose it still needs a matching \) on the same run with no paragraph break to fire. the \\{1,2} widening only adds \\(...\\), even less likely than \(...\) in non-math prose. net: low, and confined to math-delimiter-shaped text.
| <MarkdownTextSurface {...surfaceProps} /> | ||
| </DeferStreamingText> | ||
| </SmoothStreamingText> | ||
| <MarkdownTextSurface defer smooth={SMOOTH_OPTIONS} {...surfaceProps} /> |
There was a problem hiding this comment.
The pipeline reorder (smooth → defer → preprocess ⟶ preprocess → smooth → defer) is not intermediate-frame identical for reasoning text, even though the final render is. In the built-in StreamdownTextPrimitive, useSmooth runs on the already-remend-repaired full text, and the revealed prefix is then rendered with parseIncompleteMarkdown: false and no per-prefix repair.
Consequence (reasoning only — this MarkdownTextContent path with smooth; the body-text MarkdownText is defer-only and genuinely equivalent): the OLD code re-ran tailBoundedRemend on each revealed prefix, so an incomplete **bold showed up already-styled during the reveal. The NEW code reveals a prefix of the repaired text, so the opener is shown before its closer → **, backtick, $ briefly flash as literal syntax at the typewriter frontier until the reveal catches up.
Questions:
- Was this transient flicker observed during your manual reasoning-stream testing, and is it considered acceptable? (We think it's likely fine — it's cosmetic and reasoning-only — but the PR body's "end result is identical" should be narrowed to final state.)
SMOOTH_OPTIONSmatches the old constants, but the built-inTextStreamAnimatoruses a different rate algorithm than the deleteduseSmoothReveal(incl. amaxCharIntervalMsdefault of 5ms). Did the reveal cadence visibly match the old feel side-by-side, or just approximately?
There was a problem hiding this comment.
confirmed, and you're right that this is the one place "end result is identical" overclaims. it's intermediate-frame divergent on the reasoning path only (MarkdownTextContent with smooth; body text is defer-only and genuinely equivalent). the mechanism is exactly as you describe: the built-in runs preprocess on the full text, useSmooth slices a prefix of the already-repaired string, and because we pass parseMarkdownIntoBlocksFn the built-in tail-remend is gated off (!parseMarkdownIntoBlocksFn) with parseIncompleteMarkdown false, so the reveal frontier is never re-repaired and an unclosed **, backtick, or $ shows raw until its closer is revealed.
treating it as acceptable: cosmetic and reasoning-only. narrowed the PR body to final-state equivalence and fixed the misleading inline comment in fbe98ca. if we decide the flicker isn't shippable, the fix is either dropping parseMarkdownIntoBlocksFn on the smooth surface (costs the reasoning block-parse cache) or setting parseIncompleteMarkdown non-false there (full remend per flush on reasoning).
on cadence (Q2): approximately, not exact. maxCharIntervalMs is unset so it defaults to 5ms against the old ~33ms floor, so the tail reveals faster. if the feel is off we can set maxCharIntervalMs explicitly to match.
| this.repository.addOrUpdateMessage( | ||
| messages.at(-1)?.id ?? null, | ||
| fromThreadMessageLike( | ||
| { role: 'assistant', content: [], metadata: { isOptimistic: true } }, |
There was a problem hiding this comment.
The inlining of the removed appendOptimisticMessage looks faithful (generateId + fromThreadMessageLike(..., { type: 'running' }) + addOrUpdateMessage), and adding metadata.isOptimistic: true for the new off-branch eviction is a sensible, necessary adaptation rather than a gratuitous change.
This is the highest-risk spot since it's the one place we hand-reimplement a core method against a core that jumped two minors. Questions:
- Did you confirm the placeholder is correctly evicted (no ghost/empty assistant bubble) across: stop mid-stream, regenerate/reload, edit-and-resend, and branch switch? Those are the paths where a stale optimistic id historically leaks.
- Is
metadata.isOptimistica documented/stable contract in core 0.2.x's eviction logic, or an implementation detail we're relying on? If the latter, can we add a short comment pointing at the upstream code so a future core bump doesn't silently break cleanup?
There was a problem hiding this comment.
faithful, yes. on whether isOptimistic is a contract or an implementation detail: it's a documented public field, not something we're reaching past. core's published types carry the JSDoc on ThreadMessage.metadata.isOptimistic: "Marks a client-side optimistic placeholder. Such messages are evicted once off the head branch and are never persisted." it landed in #4162, and core's own external-store runtime uses the identical generateId() + fromThreadMessageLike({ ..., metadata: { isOptimistic: true } }, id, { type: 'running' }) pattern, so we're matching core's usage exactly rather than relying on an internal detail. added a short invariant note at the call site in 27f8d78 flagging that dependency (without a PR-pointer, since the contract lives in core's type) so a future core bump that touches it gets caught here.
on eviction (Q1): verified clean against the installed 0.2.18. stop bypasses core's cancelRun (our Stop wires onCancel directly to the gateway), so the placeholder is cleaned by our own sync instead: hasUpcomingMessage is false on the next snapshot, so we deleteMessage the tracked optimistic id and don't re-add it. branch-switch routes through core switchToBranch, which early-returns while running, and when idle there's no optimistic to evict; within every sync the placeholder is the on-branch head at each resetHead, so core's off-branch eviction never touches it. no double-evict or ghost bubble across stop, regenerate, edit-and-resend, or branch switch.
…ssageRenderBoundary @assistant-ui/store renamed its index-out-of-bounds throw from tapClientLookup/tapClientResource to useClientLookup in the 0.14 upgrade, so the boundary's /tapClient.../ filter stopped matching and re-threw the transient session-switch and reconnect race to root, blanking the app. broaden the regex to accept the new prefix (keeping the old one for older store versions) and point the test at the real message so it exercises the live path instead of the dead string.
…th swap the parseIncompleteMarkdown comment implied the reveal frontier is repaired; repair runs on the full accumulated text, so reword it to say that. drop the now-dead "multiple surfaces render the same content" clause from the block-cache comment (the smooth and defer wrappers that caused it were removed), and trim the math-preprocess comment to the load-bearing prose-only constraint.
…reprocess swap lock the four behaviors the built-in normalizeMathDelimiters/escapeCurrencyDollars introduce over the deleted custom helpers: $$<digit>$$ display math stays intact, double-backslash brackets and [/math]/[/inline] tag pairs rewrite to dollar delimiters, and currency dollars in prose are escaped. the existing preprocessMarkdown suite had no math cases.
…timistic placeholder a reader of this subclass can't recover from hermes code alone that the metadata.isOptimistic flag drives core's off-branch eviction and export() omission, so a future core change to it would silently break placeholder cleanup. flagged in the upgrade review.
|
@OutThisLife thanks again for the review. here's where everything landed. inline threads (all three replied + addressed):
Q1 (0.13/0.14 behavior changes beyond
none of the rest change behavior for us (we don't set Q2 (manual scenarios behind "all pass"): reproduce via
eslint: the next steps: a few of these point at deeper assistant-ui cleanup, the biggest being the subclass. now that #4415 made incremental sync native, we're keeping that, and any other tuning, out of this PR on purpose. the goal here is a smooth, low-risk transition to 0.14; bundling every optimization into one PR is how subtle regressions slip in. so the plan is to land this as the clean baseline first, then take each follow-up item one at a time in its own focused PR once this merges, rather than cram them all together here. |
|
Superseded — cherry-picked onto current |
|
Superseded — see #63970. |
…t-ui chore(desktop): upgrade @assistant-ui to 0.14 + use built-in streaming APIs (supersedes #51653)
…653-assistant-ui chore(desktop): upgrade @assistant-ui to 0.14 + use built-in streaming APIs (supersedes NousResearch#51653)
…653-assistant-ui chore(desktop): upgrade @assistant-ui to 0.14 + use built-in streaming APIs (supersedes NousResearch#51653)
…653-assistant-ui chore(desktop): upgrade @assistant-ui to 0.14 + use built-in streaming APIs (supersedes NousResearch#51653)
upgrades
@assistant-ui/reactfrom^0.12.28to^0.14.23and@assistant-ui/react-streamdownfrom^0.1.11to^0.3.4, then replaces all hand-rolled streaming-rendering code with the built-indefer,smooth,tailBoundedRemend,normalizeMathDelimiters, andescapeCurrencyDollarsAPIs that shipped in@assistant-ui/react-streamdown@0.3.x.what changed
1. dependency upgrades
@assistant-ui/react^0.12.28to^0.14.23@assistant-ui/react-streamdown^0.1.11to^0.3.4@assistant-ui/storeoverride removed from rootpackage.json(all transitive ranges now resolve to0.2.18without it)resolved tree:
2. code fix:
appendOptimisticMessageremovalMessageRepository.appendOptimisticMessage()was removed in core 0.2.x (assistant-ui#4162). the method generated a unique id, converted the message viafromThreadMessageLike, and calledaddOrUpdateMessage. this is now inlined inincremental-external-store-runtime.ts, withmetadata.isOptimistic: trueadded so the new off-branch eviction logic cleans up the placeholder correctly.3. import cleanup: internal to public
fromThreadMessageLikeandgenerateIdgraduated to the public API in 0.14.22 (assistant-ui#4414), so they now import from@assistant-ui/reactinstead of@assistant-ui/core/internal.ExportedMessageRepositoryin the test file moves to the public import for the same reason. the remaining internal imports (AssistantRuntimeImpl,BaseAssistantRuntimeCore,ExternalStoreThreadListRuntimeCore,ExternalStoreThreadRuntimeCore,hasUpcomingMessage) are runtime construction internals with no public equivalent and stay on@assistant-ui/core/internal.4. replace custom streaming wrappers with built-in
defer+smoothdeleted
SmoothStreamingText,DeferStreamingText, anduseSmoothReveal(~174 lines) frommarkdown-text.tsx. the built-indeferandsmoothprops onStreamdownTextPrimitivenow handle the same work:useDeferredValueso markdown re-parsing runs at lower priority (typing/scrolling stay responsive during streaming)useSmoothwithSmoothOptions{ drainMs: 500, maxCharsPerFrame: 30, minCommitMs: 33 }, matching the olduseSmoothRevealconstants exactlyMarkdownTextContent(reasoning text) gets bothdeferandsmooth.MarkdownText(assistant text) getsdeferonly, matching the previous behavior where text messages had no typewriter effect.the internal pipeline order changes from
smooth → defer → preprocesstopreprocess → smooth → defer(the built-in primitive runs preprocess first). final-state output is identical, but the two are not intermediate-frame identical during streaming on the reasoning (smooth) path: because a customparseMarkdownIntoBlocksFngates off the built-in tail-remend andparseIncompleteMarkdownis false, the smooth reveal slices already-repaired text, so an unclosed delimiter at the typewriter frontier shows raw until its closer is revealed. cosmetic and reasoning-only; the body-text path (defer-only) is fully equivalent.5. replace custom
lib/remend-tail.tswith built-intailBoundedRemenddeleted
lib/remend-tail.ts(108 lines) andlib/remend-tail.test.ts(105 lines). thetailBoundedRemendandfindRemendWindowStartexports from@assistant-ui/react-streamdown@0.3.4are algorithmically identical to the custom implementation — same boundary scan, same fence/math tracking, same slice-and-repair strategy. the only differences are improvements: the built-in handles\r(CR) in line endings for Windows compatibility, and accepts an optionalRemendOptionsparameter passed through toremend.6. replace custom math delimiter helpers with built-in
normalizeMathDelimiters+escapeCurrencyDollarsdeleted the custom
rewriteLatexBracketDelimitersandescapeCurrencyDollarsimplementations frommarkdown-preprocess.ts(~40 lines). the built-in exports from@assistant-ui/react-streamdown@0.3.4are strict improvements:normalizeMathDelimiterscombinesrewriteLatexBracketDelimiters(now handles double backslashes and trims body whitespace) withrewriteCustomMathTags(handles[/math]...[/math]and[/inline]...[/inline]tags that some models emit — new capability HA didn't have before)escapeCurrencyDollarsexcludes$as a preceding character, so display math$$5is no longer incorrectly escaped (bugfix)why
the custom wrappers in
markdown-text.tsx,lib/remend-tail.ts, andmarkdown-preprocess.tswere necessary on the old@assistant-ui/react-streamdown@0.1.xbecause the built-in APIs didn't exist yet. now that they ship in0.3.x, we can delete the hand-rolled implementations and use the first-party primitives. this removes ~427 lines of custom streaming code and eliminates the maintenance burden of keeping it in sync with upstream changes.what's kept
two custom optimizations remain because they have no built-in equivalent:
lib/katex-memo.ts— memoized rehype-katex plugin that caches rendered math nodes by source text, avoiding re-rendering unchanged equations on every streaming tokenparseMarkdownIntoBlocksCachedinmarkdown-text.tsx— module-level LRU cache for block parsing, avoiding repeatedmarkedlex on message remount (virtualizer scroll, session switch)review follow-ups
on top of a merge up to current
main(4b12864c2):MessageRenderBoundaryregex (24a940fa3): the 0.14@assistant-ui/storerenamed its index-out-of-bounds throw fromtapClientLookup/tapClientResourcetouseClientLookup, so the boundary's transient-error filter stopped matching and re-threw the session-switch/reconnect race to root, blanking the app. broadened the regex to accept the new prefix and pointed the test at the real message. not visible in the original diff; surfaced by the version jump.94756bd7a): thepreprocessMarkdownsuite had no math cases, so added four that lock the behavior the built-in helpers introduce:$$5x = 10$$stays intact,\\(x^2\\)rewrites,[/math]/[/inline]tag pairs rewrite, and currency dollars in prose are escaped.isOptimisticinvariant note (27f8d78a5): documented at the call site thatmetadata.isOptimisticis load-bearing for core's off-branch eviction andexport()omission.fbe98caa9): tightened the streaming-repair comments after the defer/smooth swap.verification
tsc --noEmit: 0 errorseslint: cleanvitest: 15 failed, 786 passed (801 total). all 15 failures are pre-existing (aCSS.escapeissue inthread-timeline.tsx, electron tests needing the electron environment, platform-specific path tests) and unrelated to this change.