🐢 perf: Collapse Settled Reasoning Text in AnimatedText - #16403
Conversation
AnimatedText re-split and re-classified the entire accumulated reasoning string on every streamed token and rendered one element per word, making a long reasoning stream quadratic. Classification is now append-only: the leading run of words that finished fading is committed as a single text node and never re-split, and only the unsettled tail is classified.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The settled-prefix check still rescans accumulated text on every token, leaving quadratic streaming work.
Review effort: Lite
Findings: None
What changed in this PR
This pull request optimizes streamed reasoning text rendering by collapsing settled text and classifying only the unsettled tail.
Changes:
- Tracks and renders settled prefixes as a single text node.
- Preserves growing-word and replacement handling.
- Adds regression tests for optimized rendering behavior.
| File | Summary |
|---|---|
client/src/components/Chat/Messages/Content/animate.tsx |
Implements settled-prefix optimization. |
client/src/components/Chat/Messages/Content/__tests__/animate.test.tsx |
Tests prefix collapsing and replacement behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ef1cf6519
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stateRef.current = createFadeState(); | ||
| } | ||
| const state = stateRef.current; | ||
| const settled = text.startsWith(settledRef.current) ? settledRef.current : ''; |
There was a problem hiding this comment.
For an append-only reasoning stream, text.startsWith(settledRef.current) must compare the entire accumulated settled prefix on every render. Since that prefix grows with the response, processing a full stream still performs O(n²) character work rather than work proportional to the unsettled tail, so sufficiently long reasoning can retain the jank this change is intended to remove. The ordinary append path needs a way to reuse prior append validation without rescanning the complete prefix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b46de48. Measured: full startsWith cost ~90us/token on a 36KB concatenated stream (540ms total). extendsPrefix now probes the prefix head and the 32 chars at its boundary (~3.7us/token), and the settled region renders from text.slice(0, settledLength), so a rewrite that slips the probes can affect fade timing but never the characters shown. Covered by the new rewrite case in animate.test.tsx and the long-reasoning-stream scenario.
The append check compared the whole settled prefix on every token, which kept the stream quadratic in character comparisons. It now probes the prefix head and the region at its boundary, and the settled region is rendered from the current text so a rewrite can never show stale characters. Adds a long reasoning stream scenario to the mock harness.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b46de48bf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review the current PR head b46de48. State the exact reviewed commit and ignore findings that apply only to earlier heads. Purpose supplied by the requester: Same-head round approved by the owner to confirm b46de48 after 4112779176 was disproved with measurements; no code changed since the last round |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b46de48bf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while (index > 0 && !WHITESPACE_REGEX.test(value[index - 1])) { | ||
| index -= 1; |
There was a problem hiding this comment.
At reviewed commit b46de48bf4d4366e7a505915a482dac54ca6000e, when a long reasoning response is CJK, Thai, or otherwise streamed without whitespace, lastWordStart walks back to index 0 on every render. Consequently, limit remains at the existing settled boundary and the first tail segment is always selected as the boundary, so settledRef never advances; classifyValue repeatedly re-segments and renders the entire accumulated response. This leaves the explicitly supported spaceless scripts on the quadratic path this change is intended to eliminate, so the boundary should account for the Intl.Segmenter parts rather than whitespace alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7557868. stableLimit settles everything before the final segment of a spaceless last word, so a CJK/Thai run no longer stays in the tail; re-segmenting there only changes fade granularity since parts always concatenate to the exact text. New unit test streams a CJK run and asserts it collapses into the settled text node (fails with the previous limit); long-reasoning scenario passes on desktop light, dark and mobile.
A CJK or Thai reasoning stream never reaches whitespace, so the settled prefix could not advance and the whole run was re-segmented per token. Inside a spaceless last word everything before its final segment is now treated as stable; re-segmenting there only changes fade granularity.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7557868c0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (let i = segments.length - 1; i >= 0; i--) { | ||
| if (NON_WHITESPACE_REGEX.test(segments[i].value)) { | ||
| return segments[i].start; | ||
| } |
There was a problem hiding this comment.
When Intl.Segmenter returns one segment for an entire spaceless run, this loop returns that segment's initial offset, so nextSettledLength never advances and every streamed token reclassifies the full accumulated text. This occurs for long unspaced Hangul strings (which are explicitly matched by SPACELESS_REGEX) and for every spaceless script when Intl.Segmenter is unavailable. Fresh evidence in the current head is that the new final-segment fallback still leaves these single-segment runs on the quadratic path; provide a bounded fallback boundary when no earlier segment exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 72e619f and 7e16f9f. The per-part rule is replaced by a length cap: the unsettled tail is at most 64 code units regardless of segmentation (single-segment Hangul runs, URLs, spaceless scripts), cut at the last Intl.Segmenter grapheme boundary so emoji and combining sequences stay whole. Without Intl.Segmenter there is no safe cut, so the cap is skipped and the tail stops at the last word as before. Tests measure the classified input length per render and fail on 7557868.
Settling at part boundaries left any run that never reaches whitespace (unspaced Hangul, a spaceless script without Intl.Segmenter, long URLs) in the tail, re-classified on every token. The unsettled region is now capped at 64 characters regardless of segmentation, replacing the spaceless-specific rule; only non-animated text is ever cut, so a cut inside a part is invisible. Tests measure the classified length per render instead of the DOM shape.
The length cap could cut inside a surrogate pair or a combining or ZWJ sequence, splitting one glyph across two text nodes. The cut now steps back past continuation code units.
Code-unit rules missed emoji modifiers and regional-indicator flags. The cut now takes the last grapheme boundary from Intl.Segmenter over the unsettled tail, which starts at a committed cluster boundary and is bounded by the cap; the code-unit rule remains as the fallback.
The code-unit fallback could still split emoji modifiers and flags. With no Intl.Segmenter there is no safe cut, so the cap is not applied and the tail stops at the last word, as it did before.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
# Conflicts: # e2e/setup/fake-model.js
|
Conflict resolution ready for review at ee4d125. This head merges the current dev branch and preserves both the long-reasoning and slow-thinking mock-model fixtures. Focused checks are running. |
Pull Request
Summary
While a reasoning block streams with
smoothStreamingon and the thinking panel expanded,AnimatedTextre-split and re-classified the entire accumulated reasoning string on every token and rendered one React element per word. Per-token work grew with the text length, so a long chain of thought was quadratic overall and visibly janky late in the stream.Classification is now append-only. After each commit the component keeps the leading run of words that have finished fading as one settled prefix, rendered as a single text node and never re-split. The next render only classifies the text after that prefix, continuing from its character offset, so per-token work is proportional to the unsettled tail. The prefix always stops before the last word, since the next token can still extend it, and it is published in the same layout effect that commits the fade state, so an abandoned render leaves nothing behind. If the text no longer extends the prefix, it falls back to classifying from the start.
Type of change
Testing
Tested environments/configuration:
Automated tests:
AnimatedTextcases inanimate.test.tsx: the settled prefix collapses into one text node with only the newest word fading, the last word stays outside the prefix while it grows, per-render classified length stays bounded for spaced, CJK and unsegmented runs, grapheme clusters are never split (with and withoutIntl.Segmenter), and non-extending rewrites render the current characterse2e/specs/mock/scenarios/long-reasoning-stream.spec.tsstreams a 400-word reasoning block and checks that mid-stream the paragraph holds fewer than 40 fading spans and 60 child nodes, then the exact full text; passes on desktop light, desktop dark and mobilenpx jest src/components/Chat/Messages/Contentplus the suites importinganimate.tsx: 119 suites, 1895 tests passingnpx tsc --noEmitinclient, ESLint, Prettier and import sorting cleanScreenshots / recordings
No user-facing change: the rendered text and fade spans on newly streamed words are unchanged.
Risk / compatibility
The append check probes only the prefix head and the 32 characters at its boundary instead of comparing the whole prefix per token, and the settled region always renders from the current text, so a rewrite that slips past the probes can shift fade timing but never the characters shown. The unsettled tail is capped at 64 code units so runs that never reach whitespace (CJK, Thai, unspaced Hangul, long URLs) stay bounded; the cut lands on an
Intl.Segmentergrapheme boundary so emoji and combining sequences stay whole, and only text that has finished fading is ever cut. WithoutIntl.Segmenterthe cap is skipped and the tail stops at the last word. The markdown fade path (createFadePlugin) is unchanged and tracked separately.Checklist