Skip to content

fix(chat): coalesce streaming markdown render to one animation frame - #484

Merged
Leo310 merged 2 commits into
s2b-dev:mainfrom
Direct-Launch:fix/chat-stream-render-throttle
Sep 13, 2026
Merged

Leo310 merged 2 commits into
s2b-dev:mainfrom
Direct-Launch:fix/chat-stream-render-throttle

Conversation

@Direct-Launch

Copy link
Copy Markdown
Contributor

Part of #482. MarkdownRenderer.svelte re-parsed the entire accumulated message (container.empty() + MarkdownRenderer.render) on every streamed token, forcing a full style/layout recalc per token that stalls the whole UI for the length of a reply (a DevTools trace showed 3,146 ms UpdateLayoutTree + layout thrashing, with the plugin the dominant cost).

Now coalesced to one render per requestAnimationFrame: the effect re-runs on each content change and its cleanup cancels the previous pending frame, so a burst of tokens collapses to one re-parse per frame (~60 Hz). Latest content always wins, so the final token is never dropped; the pending frame is cancelled on unmount.

Tested: real vault, reasoning model, long replies — stutter gone, final reply renders complete. bun run check clean.

AI assistance: developed with AI assistance (Claude) and tested in a real vault before submitting; disclosing per the contribution guide.

The shared MarkdownRenderer $effect tore down (container.empty()) and
re-parsed the entire accumulated message via MarkdownRenderer.render on
every `content` change. During streaming, chatStore updates content once
per token (often 100-300/s), so this ran a full re-parse + forced
style/layout recalc per token, stalling the whole Obsidian UI for the
length of a reply. A DevTools trace showed 3,146 ms UpdateLayoutTree +
layout thrashing (get offsetParent), with smart-second-brain the dominant
plugin cost (7,357 ms JS self-time).

Schedule the render on requestAnimationFrame and cancel the pending frame
in the effect cleanup, so a burst of token updates collapses to one
re-parse per frame (~60 Hz). The effect still re-runs on every content
change and always renders the newest content, so the final token is never
dropped; static (non-streaming) content simply paints one frame later.
@greptile-apps

greptile-apps Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR coalesces repeated streaming-markdown updates into one render per animation frame, reducing full-message reparsing and layout work while retaining lifecycle guards.

  • Captures all render inputs synchronously so Svelte tracks changes to content, source path, and math settings.
  • Cancels pending frames and prevents superseded or unmounted asynchronous work from mutating the container.
  • The latest change fully addresses the prior enableMath reactivity finding.

Confidence Score: 5/5

The PR appears safe to merge; the previous math-setting reactivity issue is fully fixed and no new actionable failures remain.

enableMath is now read synchronously within the effect, so changes retrigger rendering, while cleanup cancels pending frames and guards asynchronous work against stale or unmounted DOM updates.

Important Files Changed

Filename Overview
src/components/ui/MarkdownRenderer.svelte Coalesces markdown rendering through requestAnimationFrame and synchronously captures enableMath, resolving the prior reactivity concern without introducing a new actionable issue.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  U[Reactive input changes] --> E[Svelte effect]
  E --> C[Capture current render inputs]
  C --> F[Schedule animation frame]
  U --> X[Effect cleanup]
  X --> D[Mark old render disposed]
  X --> K[Cancel pending old frame]
  F --> R[Render latest markdown]
  R --> G{Still mounted and current?}
  G -- Yes --> N[Normalize rendered links]
  G -- No --> A[Abort stale work]
Loading

Reviews (2): Last reviewed commit: "fix(chat): track enableMath in the rende..." | Re-trigger Greptile

Comment thread src/components/ui/MarkdownRenderer.svelte Outdated
Address review: moving render() into requestAnimationFrame meant enableMath was
read only in the deferred callback, so a dynamic enableMath change no longer
re-ran the effect. Capture it synchronously alongside content/sourcePath.

@Leo310 Leo310 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM — this is what the trace in #482 asks for, and the lifecycle handling is careful: every dependency is read synchronously in the effect (reactivity is intact after the enableMath fix), and the cleanup both cancels the pending frame and marks in-flight async work disposed, which extends the old unmount guard to supersession.

I checked the one thing that could regress — layout reads that assume the markdown is already in the DOM. The only such path is scroll-to-latest after a send, which measures the new user bubble's top after tick(). Its position depends only on the messages above it (already painted), and the last message carries min-h-[95%], so the scroll clamp can't bite while the bubble's content is one frame late. The other consumers use a ResizeObserver or are static.

Format, lint, bun run check, and the full unit suite pass locally on this branch (the GitHub CI run needs a maintainer approval to start, which we'll do).

Two non-blocking notes, mostly for our own follow-up:

  • This is a mitigation rather than the full fix — each frame still re-parses the whole accumulated message, so a very long reply can still push a single frame past budget. Incremental/append-only rendering of the streaming message is the real answer to the first half of #482; we'll pick that up on our side after this lands.
  • The global window.requestAnimationFrame is consistent with the rest of the repo, so no change needed here.

@Leo310
Leo310 merged commit 5e2a961 into s2b-dev:main Sep 13, 2026
3 checks passed
Leo310 added a commit that referenced this pull request Sep 14, 2026
Stability release for the Obsidian 1.13 installer (Electron 43 / Node 24):
the renderer crash on the first chat send (#492) and the transport
connection error (#501), plus the streaming render coalescing (#484), the
checkpoint callbacks strip (#483), and the indexing fixes since 2.1.0
(#486, #487, #490, #491, #493, #495, #497, #498, #499, #500).

Co-Authored-By: Claude <noreply@anthropic.com>
Leo310 added a commit that referenced this pull request Sep 15, 2026
…rsing it every frame (#503)

* perf(chat): render the streaming reply incrementally

While a reply streams, MarkdownRenderer tore down and re-parsed the whole
accumulated message once per animation frame (#484 coalesced per-token
renders to per-frame, but each render was still O(length)), so a long reply
pinned the main thread for its whole duration and the entire app stuttered
(#482).

The streaming render is now split at safe block boundaries: a sealed prefix
whose DOM is never touched again, and a live tail (the block being written)
that is the only part re-parsed per frame. `findSealableEnd` picks cut points
outside fences and math blocks, before a complete line, and never inside a
loose list, an indented continuation or a blockquote run. Outside streaming,
and when the reply settles, the content is still rendered as one document.

On a 6000-word reply streamed at 4 ms/token: long tasks 60 -> 2, frames over
50 ms 136 -> 3, DOM nodes removed 431k -> 7.7k, and the stream no longer
stalls (41 s -> 29 s wall for a 29 s stream).

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(chat): match closing fences exactly and ignore inline $$ when sealing

Review findings on #503: any line with three backticks or tildes toggled the
fence state, so a shorter or mismatched marker inside a longer fence was taken
as its close and a blank line inside the code could be sealed; and every `$$`
on a line toggled the math state, so `` `$$` `` or `costs $$5` flipped it
permanently and disabled sealing for the rest of the reply. A fence now
closes only on the opening marker's character, at least as long, with no
info string; a math block only opens at a line start and closes on a line
ending with `$$`.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(chat): a backtick fence opener cannot carry a backtick in its info string

Review finding on #503: ````foo`bar` is ordinary text under CommonMark, but the
scanner recorded it as an opening fence and, without a matching close,
suppressed every later cut point for the rest of the reply.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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