Repository navigation
perf(coding-agent): memoize syntax highlighting and fix stale render caches - #319
Merged
Merged
Conversation
liuedcson
requested review from
MelodyVAR,
Uking-xxx and
ZouR-Ma
as code owners
October 28, 2026 05:47
liuedcson
force-pushed
the
perf/tui-lexer-and-transform-prefilter
branch
from
October 28, 2026 07:56
df7e5d0 to
9ff5096
Compare
Two caches in the markdown renderer were not tied to the inputs they derive from. `defaultStylePrefix` is the ANSI prefix appended after every inline span that ends inside a style. It is derived from the theme, but `invalidate()` — which is how a theme or settings change reaches a live component — did not clear it, so a switch that stops applying a style spliced the previous theme's escape into the new output. Visible once the new theme drops the style: with bold turned off, `a line with **bold** inside it` renders as `bold\u001b[1m inside it`, with a bold run that never terminates in the theme's own terms. `setIncrementalRenderDisabled(true)` is documented and exported as the seam that gives the render-equivalence harness an unoptimized path, and every other component honours it — `Container`, `Spacer`, `Box`, `Loader`, the screen diff, and the assistant message's zone cache. `Markdown` did not, so the "full render" baseline it produced was still reusing a certified prefix. It now renders the document whole and retains nothing while the switch is set. `packages/tui/test/markdown-invalidate.test.ts` covers both: a theme switch must match a freshly constructed instance (before the fix, one line of the wrapped document keeps the stale escape), and with the switch set the renderer must lex every frame with no prefix reuse while still matching a cold render. Reverting either fix fails its test. The same file's fixture list also gains the block shapes that the per-token cache investigation contributed: a balanced inline anchor followed by an autolink, an ordered list starting at 3, a loose list, a setext `=` heading, a nested blockquote, CJK with emoji and full-width punctuation, inline LaTeX, and a long single paragraph. Each is checked against a cold render at every prefix and three widths, like the rest of the suite.
…t and source A streamed reply re-renders the same fenced blocks over and over. The markdown renderer treats a theme as a set of pure functions, so it cannot deduplicate the work that happens inside one: every frame that re-renders a finished code block highlights it again from scratch. Measured with the production theme and the real highlight.js pipeline, 40 KB at 120 columns over 120-byte deltas: | document | before | after | | --- | --- | --- | | block math (renders whole every frame) | 1 154 ms | 211 ms | 5.5x | | reference definition (renders whole every frame) | 1 206 ms | 237 ms | 5.1x | | prose containing a fence | 29 ms | 9 ms | 3.2x | | prose with no fence | 10 ms | 9 ms | 1.15x | The last row is the honest bound: a document without a fence never calls the highlighter, so the memo cannot help it and does not claim to. The cache key has three parts, and each one is load-bearing: - **The resolved theme instance.** `theme` is a proxy whose identity never changes, so keying on it would serve the previous theme's colours forever. Every replacement funnels through `setGlobalTheme`, which stores the backing instance under a `Symbol.for` key. - **The grammar set revision.** `highlight.js` resolves embedded sub-languages through one registry, so the same source and language highlight differently before and after the remaining grammars load: a JavaScript template literal only gets HTML colouring once the XML grammar exists. The product loads those grammars after startup (`loadAllHighlightLanguages`), so a block highlighted during the first frames would otherwise keep its pre-load rendering for the rest of the session. The revision is bumped when the loader resolves, which retires the old keys. - **Language and source.** The same text highlights differently as TypeScript and as JSON. Two bounds keep it from becoming the leak it exists to avoid: - **Stored on the second request, not the first.** A fence that is still being written produces a new key on every delta, and each of those prefixes is dead as soon as the next one arrives. Caching them filled the budget with hundreds of intermediate versions of a single block (measured: 33 MiB of heap for one 56 KB fence) and evicted the finished blocks the memo is for. A finished block repeats its key, so it is stored on the second miss. The same stream now retains 1.2 MiB. - **Bounded by payload, not entry count** (4 MiB per theme, FIFO), and sources above 64 Ki UTF-16 code units are never retained, so whole-file callers (read and write tools) cannot pin a file in memory. Every result is a copy: `core/tools/write.ts` keeps the returned array and appends to or overwrites its elements while a file streams, so handing out the cached array would corrupt the cache. Both entry points (`highlightCode` and the markdown theme's `highlightCode`) write into the same memo, so their `catch` fallbacks now agree. Before, an exception left one path with plain text and the other with `mdCodeBlock` colouring, and the result depended on which caller ran first. `packages/coding-agent/test/theme-highlight-cache.test.ts` covers the cases where an entry could be served for the wrong inputs or outlive its budget: a theme switch (labelled and unlabelled fences), the grammar revision, an explicitly passed theme not sharing entries with the global one, a mutating caller not reaching the cache, language in the key, one-off streaming keys not consuming the budget, a repeated block being served from it, eviction by payload, the source limit, and agreement between the two entry points including the failure fallback. Each guard was checked by mutating the implementation: storing every miss fails two tests, dropping the grammar revision fails one, removing the payload budget fails one, keying on the proxy fails two, handing out the cached array fails one, and dropping the `defaultStylePrefix` reset fails two. Rebased onto the tree that already contains #315; the earlier per-token render cache is not part of this change, because after #315 it only paid where that PR falls back to a whole-document render.
liuedcson
force-pushed
the
perf/tui-lexer-and-transform-prefilter
branch
from
October 28, 2026 08:50
9ff5096 to
ff80402
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
Two caches that outlived the inputs they were derived from, plus the repeated syntax highlighting that a streaming re-render still pays for.
This is a narrow follow-up to #315. #315 made a streamed reply reuse a certified prefix, so an ordinary message no longer re-lexes, re-renders or re-wraps finished blocks. What #315 left behind is the work that happens inside a re-rendered block, and two caches that
invalidate()did not cover.1. Syntax highlighting was recomputed for every re-rendered block
The markdown renderer treats a theme as a set of pure functions, so it cannot deduplicate the work that happens inside one. Every frame that re-renders a finished code block highlights it again from scratch.
Measured with the production theme and the real highlight.js pipeline, 40 KB at 120 columns over 120-byte deltas:
The last row is the honest bound: a document without a fence never calls the highlighter, so the memo cannot help it and does not claim to.
The key has three parts, and each is load-bearing.
themeis a proxy whose identity never changes, so keying on it would serve the previous theme's colours forever. Every replacement funnels throughsetGlobalTheme, which stores the backing instance under aSymbol.forkey.highlight.jsresolves embedded sub-languages through one registry, so the same source and language highlight differently before and after the remaining grammars load — a JavaScript template literal only gets HTML colouring once the XML grammar exists. The product loads those grammars after startup, so a block highlighted during the first frames would otherwise keep its pre-load rendering for the rest of the session.loadAllHighlightLanguages()bumps the revision, which retires the old keys.Two bounds keep the memo from becoming the leak it exists to avoid.
Every result is a copy:
core/tools/write.tskeeps the returned array and appends to or overwrites its elements while a file streams, so handing out the cached array would corrupt the cache.Both entry points write into the same memo, so their
catchfallbacks now agree. Before, an exception left one path with plain text and the other withmdCodeBlockcolouring, and the result depended on which caller ran first.2.
invalidate()left the derived style prefix behinddefaultStylePrefixis the ANSI prefix appended after every inline span that ends inside a style. It is derived from the theme, butinvalidate()— which is how a theme or settings change reaches a live component — did not clear it. A switch that stops applying a style spliced the previous theme's escape into the new output. Visible once the new theme drops that style: with bold turned off,a line with **bold** inside itrenders with a bold run that the new theme never asked for and never closes.3.
Markdownignored the render-equivalence switchsetIncrementalRenderDisabled(true)is exported and documented as the seam that gives the render-equivalence harness an unoptimized path, and every other component honours it —Container,Spacer,Box,Loader, the screen diff, and the assistant message's zone cache.Markdowndid not, so the "full render" baseline it produced was still reusing a certified prefix. It now renders the document whole and retains nothing while the switch is set, and re-arms reuse when the switch is cleared.Tests
packages/coding-agent/test/theme-highlight-cache.test.tscovers the cases where an entry could be served for the wrong inputs or outlive its budget: a theme switch (labelled and unlabelled fences), the grammar revision, an explicitly passed theme not sharing entries with the global one, a mutating caller not reaching the cache, language in the key, one-off streaming keys not consuming the budget, a repeated block being served from it, eviction by payload, the source limit, and agreement between the two entry points including the failure fallback.Each guard was checked by mutating the implementation:
defaultStylePrefixresetsetIncrementalRenderDisabledbranchpackages/tui/test/markdown-invalidate.test.tscovers the two cache fixes, and the prefix-equivalence suite gains the block shapes that came out of this investigation: a balanced inline anchor followed by an autolink, an ordered list starting at 3, a loose list, a setext=heading, a nested blockquote, CJK with emoji and full-width punctuation, inline LaTeX, and a long single paragraph.Validation
packages/tui:node --test test/*.test.ts— 1058 passpackages/coding-agent:theme-highlight-cache,step-theme,syntax-highlight— 28 passapps/cli:assistant-message,step-message,mermaid, acceptance snapshots/interactions — 74 passcheck:ts-imports,check:layer-direction,check:ui-layer,check:workspace-registry,check:tui-no-ai,check:public-boundary,check:pinned-deps,check:no-observability,check:contracts-deps-empty,check:derived-compat-only— passgit diff --check— cleanScope
Rebased onto the tree that already contains #315 and #318.
This PR does not contain a per-token render cache. An earlier revision had one; after #315 it only paid where #315 falls back to a whole-document render (1.1x–1.5x), and it cost a second cache keyed on parsed subtrees. The highlight memo above covers the same documents with a larger, measured effect for a fraction of the code.
Two limits are stated rather than fixed here.
resolveThemeInstancerecognises this module instance's proxy; a proxy exported by a different module instance (tsx and jiti both loading the file) is a distinct object and would pin that instance's colours. No in-repo caller passes one — themes reach the cache through the default parameter or as a concreteTheme. And the remaining costs listed indocs/tui-rendering-pipeline.md§4.2 are unchanged: the whole-document fallback for block math and reference definitions, no certified cut for a single unbounded paragraph or code block, the per-frame pointer copy when assembling the flat line array, and Step presentation's own per-frame projection inrenderNativeRun.