From c41597c64d08d739f21448022a9c12fdd8d18b20 Mon Sep 17 00:00:00 2001 From: liuedcson <332840681+liuedcson@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:55:28 +0800 Subject: [PATCH 1/2] fix(tui): reset the derived style prefix and honour the render switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/tui-rendering-pipeline.md | 4 + packages/tui/src/components/markdown.ts | 17 ++- packages/tui/test/markdown-invalidate.test.ts | 129 ++++++++++++++++++ .../tui/test/markdown-stream-prefix.test.ts | 15 ++ 4 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 packages/tui/test/markdown-invalidate.test.ts diff --git a/docs/tui-rendering-pipeline.md b/docs/tui-rendering-pipeline.md index c7ccc0eea..faca96edd 100644 --- a/docs/tui-rendering-pipeline.md +++ b/docs/tui-rendering-pipeline.md @@ -162,6 +162,10 @@ 调用方必须复用同一个 `Markdown` 实例，否则实例级状态随组件一起消失；`AssistantMessageComponent` 因此按内容种类保留实例并只调 `setText`（`markdownSlots`）。每帧仍要做一次指针复制拼出扁平数组，以及主屏/全屏自己的差分；这两项是独立成本，不随前缀复用消失。Step 呈现的包装层（`step-message.ts` 的 `renderNativeRun`：`hideCompleteFences`、空行折叠、`trimOuterRows`）同样每帧扫一遍全部 native 行，那部分成本不受这里影响，需要单独处理。 +`invalidate()` 同时清掉 `defaultStylePrefix`。它是从主题派生的 ANSI 前缀，会在每个"在样式内结束"的行内片段之后重新拼一次；主题切换若只清缓存而留着它，旧主题的转义就会拼进新输出——当新主题不再应用该样式时（例如关掉粗体）会直接显示错。第 8 节的 `setIncrementalRenderDisabled(true)` 对这里同样生效：置位后渲染整篇且不保留前缀（`renderContent` 开头），否则那个"强制全量路径做字节级等价校验"的开关在 Markdown 内部形同虚设。 + +**语法高亮按主题实例 + `(lang, source)` 记忆化**（`cachedHighlight`，`packages/coding-agent/src/theme/theme.ts`）。渲染器把主题当纯函数用，自己无法去重，所以只要一帧里重渲了某个已完成代码块，高亮就整块重跑：认证前缀之外的路径（块级数学、reference definition）每帧重渲整篇，而认证路径的可变尾块里若含围栏也会每帧重渲。实测 60 KB、120 列、120 B/delta，用产品主题和真实 highlight.js：块级数学文档 2 687 ms → 448 ms（6.0 倍），reference definition 2 672 ms → 457 ms（5.8 倍），普通散文 45 ms → 15 ms（3.1 倍）。未标注语言的围栏同样在缓存范围内（逐行 `fg` 也不便宜）。缓存以 `Symbol.for` 解析出的真实主题实例为键（导出的 `theme` 是 Proxy、身份不变，直接用会把上一个主题的颜色永久留给下一个），按插入顺序 FIFO 淘汰、单条超过 64 KiB 不缓存，且永远返回副本——`core/tools/write.ts` 会在流式写入时持有并修改返回数组。 + ## 5. 主屏差分渲染（regular） `TuiMainScreen.doRender`（`tui-main-screen.ts:329-750`）每帧的完整顺序： diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index cdb0b6a47..4899827d4 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -1,7 +1,7 @@ import { Marked, type Token, Tokenizer, type TokenizerExtension, type Tokens, type TokensList } from "marked"; import { renderLatex } from "../latex.ts"; import { getCapabilities, hyperlink, isImageLine } from "../terminal-image.ts"; -import type { Component } from "../tui.ts"; +import { type Component, isIncrementalRenderDisabled } from "../tui.ts"; import { applyBackgroundToLine, truncateToWidth, visibleWidth, wrapTextWithAnsi } from "../utils.ts"; import { applyInlineTagDepth, @@ -322,6 +322,11 @@ export class Markdown implements Component { this.cachedText = undefined; this.cachedWidth = undefined; this.cachedLines = undefined; + // The default style prefix is derived from the theme, and a theme switch is + // delivered as an invalidate. Keeping it would splice the previous theme's + // escapes into every inline span that ends inside a style, which is visible + // whenever the new theme stops applying that style. + this.defaultStylePrefix = undefined; // Theme, width, and option changes make previously rendered rows wrong, not // just stale, so the certified prefix cannot survive an invalidate. this.resetStream(); @@ -383,6 +388,16 @@ export class Markdown implements Component { * that force a full re-render. */ private renderContent(normalizedText: string, contentWidth: number, width: number): string[] { + if (isIncrementalRenderDisabled()) { + // The shared switch promises an unoptimized path for the render-equivalence + // harness. Reusing a prefix here would make that path incremental again, so + // the document is rendered whole and nothing is retained. + this.resetStream(); + this.previousSource = normalizedText; + this.layoutWidth = contentWidth; + return this.renderFrom(normalizedText, contentWidth, width, 0, false); + } + const appendOnly = this.previousSource.length > 0 && normalizedText.length >= this.previousSource.length && diff --git a/packages/tui/test/markdown-invalidate.test.ts b/packages/tui/test/markdown-invalidate.test.ts new file mode 100644 index 000000000..2d6548a8b --- /dev/null +++ b/packages/tui/test/markdown-invalidate.test.ts @@ -0,0 +1,129 @@ +import assert from "node:assert"; +import { afterEach, describe, it } from "node:test"; +import { type DefaultTextStyle, Markdown, type MarkdownTheme } from "../src/components/markdown.ts"; +import { markdownWork, resetMarkdownWork } from "../src/components/markdown-work.ts"; +import { setIncrementalRenderDisabled } from "../src/tui.ts"; +import { defaultMarkdownTheme } from "./test-themes.ts"; + +/** + * Renderer invalidation and the shared incremental-render switch. + * + * `invalidate()` is how a theme or settings change reaches a live component, so + * every cache derived from the previous inputs has to go with it. The switch is + * the seam that promises an unoptimized path for render-equivalence checks, which + * only works if the renderer honours it too. + */ + +const identity = (text: string) => text; + +/** A theme whose emphasis can be turned off, like a theme switch that drops a style. */ +function mutableTheme(state: { bold: boolean }): MarkdownTheme { + return { + heading: identity, + link: identity, + linkUrl: identity, + code: identity, + codeBlock: identity, + codeBlockBorder: identity, + quote: identity, + quoteBorder: identity, + hr: identity, + listBullet: identity, + italic: identity, + strikethrough: identity, + underline: identity, + bold: (text) => (state.bold ? `\x1b[1m${text}\x1b[22m` : text), + }; +} + +/** A row long enough to wrap, so the default style is applied inside wrapped text. */ +const WRAPPING_DOCUMENT = "a line with **bold** inside it that is long enough to wrap at forty columns\n\n"; + +afterEach(() => { + setIncrementalRenderDisabled(false); +}); + +describe("Markdown invalidation", () => { + it("drops the default style prefix when the theme stops applying it", () => { + // The prefix is appended to every inline span that ends inside a style. A stale + // one splices the previous theme's escape into the new output, which is visible + // as soon as the new theme no longer applies that style itself. + const state = { bold: true }; + const defaultTextStyle: DefaultTextStyle = { bold: true }; + const markdown = new Markdown(WRAPPING_DOCUMENT, 0, 0, mutableTheme(state), defaultTextStyle, {}); + const before = markdown.render(40); + assert.ok(before[0]?.includes("\u001b[1m"), "the bold style never applied"); + + state.bold = false; + markdown.invalidate(); + + const expected = new Markdown(WRAPPING_DOCUMENT, 0, 0, mutableTheme(state), defaultTextStyle, {}).render(40); + assert.deepStrictEqual(markdown.render(40), expected); + }); + + it("still matches a cold render after a theme switch on a long document", () => { + const state = { bold: true }; + const defaultTextStyle: DefaultTextStyle = { bold: true }; + const document = `${WRAPPING_DOCUMENT}| a | b |\n| - | - |\n| **x** | y |\n\n# heading\n\nmore **bold** text here\n\n`; + const markdown = new Markdown(document, 0, 0, mutableTheme(state), defaultTextStyle, {}); + markdown.render(40); + for (let end = 1; end <= document.length; end++) { + markdown.setText(document.slice(0, end)); + markdown.render(40); + } + + state.bold = false; + markdown.invalidate(); + + assert.deepStrictEqual( + markdown.render(40), + new Markdown(document, 0, 0, mutableTheme(state), defaultTextStyle, {}).render(40), + ); + }); +}); + +describe("incremental render switch", () => { + it("renders the whole document and retains no prefix while disabled", () => { + const document = `${"first paragraph of prose\n\nsecond paragraph of prose\n\n".repeat(40)}`; + setIncrementalRenderDisabled(true); + const markdown = new Markdown("", 0, 0, defaultMarkdownTheme, undefined, {}); + resetMarkdownWork(); + + let frames = 0; + for (let end = 120; end < document.length; end += 120) { + frames++; + markdown.setText(document.slice(0, end)); + markdown.render(60); + } + markdown.setText(document); + const lines = markdown.render(60); + + assert.strictEqual(markdownWork.reusedFrames, 0, "a prefix was reused while the switch was set"); + assert.ok( + markdownWork.lexUnits >= frames * 120, + `only ${markdownWork.lexUnits} units were lexed across ${frames} frames, so frames were skipped`, + ); + assert.deepStrictEqual(lines, new Markdown(document, 0, 0, defaultMarkdownTheme, undefined, {}).render(60)); + }); + + it("resumes prefix reuse once the switch is cleared", () => { + const document = "first paragraph of prose\n\nsecond paragraph of prose\n\n".repeat(20); + setIncrementalRenderDisabled(true); + const markdown = new Markdown("", 0, 0, defaultMarkdownTheme, undefined, {}); + markdown.setText(document.slice(0, 200)); + markdown.render(60); + + setIncrementalRenderDisabled(false); + resetMarkdownWork(); + for (let end = 200; end < document.length; end += 60) { + markdown.setText(document.slice(0, end)); + markdown.render(60); + } + markdown.setText(document); + assert.deepStrictEqual( + markdown.render(60), + new Markdown(document, 0, 0, defaultMarkdownTheme, undefined, {}).render(60), + ); + assert.ok(markdownWork.reusedFrames > 0, "the renderer never re-armed prefix reuse"); + }); +}); diff --git a/packages/tui/test/markdown-stream-prefix.test.ts b/packages/tui/test/markdown-stream-prefix.test.ts index 786b542c0..36efdb874 100644 --- a/packages/tui/test/markdown-stream-prefix.test.ts +++ b/packages/tui/test/markdown-stream-prefix.test.ts @@ -101,6 +101,21 @@ const HAZARDS: ReadonlyArray = [ ["paragraph-continuation", "intro\n\nfirst line\nsecond line\n\nthird\n"], ["image-line", "intro\n\n![alt](https://example.com/x.png)\n\ntail\n"], ["empty-first-block", "\n\nfirst\n\nsecond\n"], + // The block shapes below were contributed by the per-token cache investigation. + // They are not new constructs, but each one stresses a different part of the + // certification: link state that is balanced again, list numbering that starts off + // one, wide characters, and nested containers. + [ + "inline-anchor-balanced-then-autolink", + 'open\n\nhttps://example.com\n\nclose \n\nhttps://example.com\n\nend\n', + ], + ["ordered-list-starts-at-three", "3. three\n4. four\n\n5. five\n\n- a\n 1. x\n 2. y\n"], + ["loose-list", "- one\n- two\n\n- three\n"], + ["setext-equals", "para\n\nheading\n===\n\nafter\n"], + ["nested-blockquote", "a\n\n> quoted\n>\n> > deeper\n>\n> more\n\noutro\n"], + ["cjk-and-emoji", "标题\n\n中文段落 🎉 全角。\n\n- 列表\n\n| 甲 | 乙 |\n| - | - |\n| 1 | 2 |\n"], + ["inline-latex", "a\n\ninline $E = mc^2$ math\n\nmore\n"], + ["long-paragraph", `word ${"word ".repeat(120)}\n`], ]; describe("Markdown streaming", () => { From ff80402078de7513fd29223c6fbb7f36c64f19b5 Mon Sep 17 00:00:00 2001 From: liuedcson <332840681+liuedcson@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:55:31 +0800 Subject: [PATCH 2/2] perf(coding-agent): memoize syntax highlighting per theme, grammar set 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. --- docs/tui-rendering-pipeline.md | 15 +- packages/coding-agent/src/theme/theme.ts | 214 +++++++++++++--- .../src/utils/syntax-highlight.ts | 20 +- .../test/theme-highlight-cache.test.ts | 232 ++++++++++++++++++ .../tui/test/markdown-stream-prefix.test.ts | 7 +- 5 files changed, 449 insertions(+), 39 deletions(-) create mode 100644 packages/coding-agent/test/theme-highlight-cache.test.ts diff --git a/docs/tui-rendering-pipeline.md b/docs/tui-rendering-pipeline.md index faca96edd..1a67faba6 100644 --- a/docs/tui-rendering-pipeline.md +++ b/docs/tui-rendering-pipeline.md @@ -164,7 +164,20 @@ `invalidate()` 同时清掉 `defaultStylePrefix`。它是从主题派生的 ANSI 前缀，会在每个"在样式内结束"的行内片段之后重新拼一次；主题切换若只清缓存而留着它，旧主题的转义就会拼进新输出——当新主题不再应用该样式时（例如关掉粗体）会直接显示错。第 8 节的 `setIncrementalRenderDisabled(true)` 对这里同样生效：置位后渲染整篇且不保留前缀（`renderContent` 开头），否则那个"强制全量路径做字节级等价校验"的开关在 Markdown 内部形同虚设。 -**语法高亮按主题实例 + `(lang, source)` 记忆化**（`cachedHighlight`，`packages/coding-agent/src/theme/theme.ts`）。渲染器把主题当纯函数用，自己无法去重，所以只要一帧里重渲了某个已完成代码块，高亮就整块重跑：认证前缀之外的路径（块级数学、reference definition）每帧重渲整篇，而认证路径的可变尾块里若含围栏也会每帧重渲。实测 60 KB、120 列、120 B/delta，用产品主题和真实 highlight.js：块级数学文档 2 687 ms → 448 ms（6.0 倍），reference definition 2 672 ms → 457 ms（5.8 倍），普通散文 45 ms → 15 ms（3.1 倍）。未标注语言的围栏同样在缓存范围内（逐行 `fg` 也不便宜）。缓存以 `Symbol.for` 解析出的真实主题实例为键（导出的 `theme` 是 Proxy、身份不变，直接用会把上一个主题的颜色永久留给下一个），按插入顺序 FIFO 淘汰、单条超过 64 KiB 不缓存，且永远返回副本——`core/tools/write.ts` 会在流式写入时持有并修改返回数组。 +**语法高亮按主题实例 + 语法版本 + `(lang, source)` 记忆化**（`cachedHighlight`，`packages/coding-agent/src/theme/theme.ts`）。渲染器把主题当纯函数用，自己无法去重，所以只要一帧里重渲了某个已完成代码块，高亮就整块重跑：认证前缀之外的路径（块级数学、reference definition）每帧重渲整篇，而认证路径的可变尾块里若含围栏也会每帧重渲。实测 40 KB、120 列、120 B/delta，用产品主题和真实 highlight.js：块级数学文档 1 154 ms → 211 ms（5.5 倍），reference definition 1 206 ms → 237 ms（5.1 倍），含一个围栏的散文 29 ms → 9 ms（3.2 倍）；**纯散文只有 1.15 倍**——不含围栏的文档本来就不调用高亮，缓存对它无用，这不是本改动的收益来源。 + +三个键成分都不能省： + +- **主题实例**：导出的 `theme` 是 Proxy、身份不变，直接用它做键会把上一个主题的颜色永久留给下一个；键取 `setGlobalTheme` 存进 `Symbol.for` 槽的真实实例。这个识别只覆盖本模块的 Proxy，跨模块实例传入的 Proxy 会被当作普通对象——仓库内没有这种调用。 +- **语法版本**（`highlightGrammarRevision`）：`highlight.js` 的子语言解析走同一个注册表，同一段 `javascript` 里的 HTML 模板只有 XML 语法加载完成后才会上色，所以 `(lang, source)` 不足以标识结果。启动时 `loadAllHighlightLanguages()` 完成后版本自增，旧键自动失效。 +- **language + source**：同一段文本按 TypeScript 和按 JSON 高亮结果不同。 + +其它约束： + +- **只在第二次请求时写入**。仍在流式增长的围栏每个 delta 都是一个新键，且这些前缀之后不会再被请求；把它们存下来会用一个代码块的几百个中间态占满预算（实测单个 56 KB 围栏 33 MiB）并把真正可复用的已完成块挤出去。已完成块会重复请求同一键，因此第二次命中才落盘。 +- **按 payload 预算淘汰**（4 MiB/主题，FIFO）而不是条目数；单条源码超过 64 Ki（UTF-16 code unit，不是字节）不保留，避免 read/write 工具把整文件钉在缓存里。 +- **永远返回副本**：`core/tools/write.ts` 会在流式写入时持有并修改返回数组。 +- 两个入口（`highlightCode` 与 markdown 主题的 `highlightCode`）共用一份缓存，因此失败兜底也统一成 `fg("mdCodeBlock", …)`；否则同一键的结果取决于谁先跑。 ## 5. 主屏差分渲染（regular） diff --git a/packages/coding-agent/src/theme/theme.ts b/packages/coding-agent/src/theme/theme.ts index 473ad51b7..24ec0510d 100644 --- a/packages/coding-agent/src/theme/theme.ts +++ b/packages/coding-agent/src/theme/theme.ts @@ -15,7 +15,7 @@ import { Compile } from "typebox/compile"; import { getCustomThemesDir, getThemesDir } from "../config.ts"; import type { SourceInfo } from "../core/source-info.ts"; import { closeWatcher, watchWithErrorHandler } from "../utils/fs-watch.ts"; -import { highlight, supportsLanguage } from "../utils/syntax-highlight.ts"; +import { highlight, highlightGrammarRevision, supportsLanguage } from "../utils/syntax-highlight.ts"; import { stripBom } from "../utils/text.ts"; // ============================================================================ @@ -1220,6 +1220,148 @@ function getCliHighlightTheme(t: Theme): CliHighlightTheme { return cachedCliHighlightTheme; } +/** + * The concrete `Theme` behind the exported proxy. + * + * `theme` is a proxy whose identity never changes, so keying a cache on it would + * survive a theme switch and serve colours from the previous theme. Every + * replacement path funnels through `setGlobalTheme`, which stores the backing + * instance under a `Symbol.for` key, so resolving it here gives the cache a key + * that changes when the theme does. + * + * This recognises *this* module's proxy. A proxy exported by a different module + * instance (tsx and jiti both loading this file) is a distinct object and is used + * as-is, so a caller that passes a foreign instance's `theme` into + * `getMarkdownTheme` would pin that instance's colours. No in-repo caller does + * that; themes reach the cache either through the default parameter or as a + * concrete `Theme`. + */ +function resolveThemeInstance(t: Theme): Theme { + if ((t as unknown) !== (theme as unknown)) return t; + const store = globalThis as Record; + return store[THEME_KEY_NEW] ?? store[THEME_KEY] ?? store[THEME_KEY_OLD] ?? t; +} + +/** + * Highlighted lines per theme, memoized on language and source. + * + * Both the streaming renderer and the tool-output path ask for the same fenced + * block repeatedly: a block is highlighted while it is being written, and any + * document that still re-renders whole (block math, reference definitions, a + * message that has not settled) re-highlights every finished block on every + * frame. The renderer cannot deduplicate that by itself because the work happens + * inside this theme object, which it treats as a set of pure functions. + * + * Only blocks that are asked for twice are retained. A fenced block that is still + * streaming produces a new key on every delta, and each of those prefixes is dead + * the moment the next one arrives: caching them would fill the whole budget with + * hundreds of intermediate versions of one block (measured: 33 MiB for a single + * 56 KB fence) and evict the finished blocks the cache exists to serve. A block + * that is merely re-rendered repeats its key, so it is stored on the second miss. + * + * The result is always 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. + */ +interface HighlightEntry { + lines: string[]; + /** Estimated payload, used for the total budget rather than an entry count. */ + bytes: number; +} + +/** Total retained payload per theme; a long answer stays well inside this. */ +const HIGHLIGHT_CACHE_BYTES = 4 * 1024 * 1024; +/** Whole-file callers (read/write tools) must not pin megabytes per entry. */ +const HIGHLIGHT_CACHE_SOURCE_LIMIT = 64 * 1024; +/** Recently requested keys, by digest, so a repeat can be told from a one-off. */ +const HIGHLIGHT_PENDING_LIMIT = 128; + +/** Bumped by `clearHighlightCache`; a state with an older generation is dropped. */ +let highlightGeneration = 0; + +/** Per-theme bookkeeping for the "store on second miss" rule. */ +interface HighlightState { + generation: number; + entries: Map; + bytes: number; + /** Digests requested once and not stored yet, oldest first. */ + recentMisses: Set; +} + +const highlightState = new WeakMap(); + +/** FNV-1a over the key; collisions only cost one extra stored entry. */ +function highlightDigest(text: string): string { + let hash = 0x811c9dc5; + for (let index = 0; index < text.length; index++) { + hash ^= text.charCodeAt(index); + hash = Math.imul(hash, 0x01000193); + } + return (hash >>> 0).toString(16); +} + +function highlightStateFor(themeInstance: Theme): HighlightState { + const existing = highlightState.get(themeInstance); + if (existing && existing.generation === highlightGeneration) return existing; + // A stale generation is dropped rather than swept, so invalidation stays O(1) + // even though the weak map cannot be iterated. + const state: HighlightState = { + generation: highlightGeneration, + entries: new Map(), + bytes: 0, + recentMisses: new Set(), + }; + highlightState.set(themeInstance, state); + return state; +} + +function cachedHighlight(t: Theme, code: string, lang: string, compute: () => string[]): string[] { + const state = highlightStateFor(resolveThemeInstance(t)); + const key = `${highlightGrammarRevision()}\u0001${lang}\u0000${code}`; + const hit = state.entries.get(key); + if (hit) return [...hit.lines]; + + // A streaming block yields a fresh key per delta and is never asked for again; + // a finished block repeats. Only the latter is worth the memory. + const digest = highlightDigest(key); + const repeat = state.recentMisses.delete(digest); + if (!repeat) { + if (state.recentMisses.size >= HIGHLIGHT_PENDING_LIMIT) { + const oldest = state.recentMisses.values().next().value; + if (oldest !== undefined) state.recentMisses.delete(oldest); + } + state.recentMisses.add(digest); + } + + const lines = compute(); + if (!repeat || code.length > HIGHLIGHT_CACHE_SOURCE_LIMIT) return [...lines]; + + const bytes = code.length + lines.reduce((total, line) => total + line.length, 0); + // Bound retained payload rather than entry count: one long block would + // otherwise outgrow any fixed number of entries. + while (state.bytes + bytes > HIGHLIGHT_CACHE_BYTES && state.entries.size > 0) { + const oldest = state.entries.keys().next().value; + if (oldest === undefined) break; + const evicted = state.entries.get(oldest); + if (evicted) state.bytes -= evicted.bytes; + state.entries.delete(oldest); + } + state.entries.set(key, { lines, bytes }); + state.bytes += bytes; + return [...lines]; +} + +/** + * Drop every memoized block for every theme. + * + * Used when the grammar set changes, and by tests that need a cold memo. State is + * invalidated by generation rather than deleted, because the per-theme map is weak + * and cannot be swept. + */ +export function clearHighlightCache(): void { + highlightGeneration += 1; +} + /** * Highlight code with syntax coloring based on file extension or language. * Returns array of highlighted lines. @@ -1227,22 +1369,26 @@ function getCliHighlightTheme(t: Theme): CliHighlightTheme { export function highlightCode(code: string, lang?: string): string[] { // Validate language before highlighting to avoid stderr spam from cli-highlight const validLang = lang && supportsLanguage(lang) ? lang : undefined; - // Skip highlighting when no valid language is specified. cli-highlight's - // auto-detection is unreliable and can misidentify prose as AppleScript, - // LiveCodeServer, etc., coloring random English words as keywords. - if (!validLang) { - return code.split("\n").map((line) => theme.fg("mdCodeBlock", line)); - } - const opts = { - language: validLang, - ignoreIllegals: true, - theme: getCliHighlightTheme(theme), - }; - try { - return highlight(code, opts).split("\n"); - } catch { - return code.split("\n"); - } + // No language means no highlighting at all, so the memo covers plain fences too: + // an unlabelled block is the most common kind, and it is restyled line by line. + return cachedHighlight(theme, code, validLang ?? "", () => { + if (!validLang) { + return code.split("\n").map((line) => theme.fg("mdCodeBlock", line)); + } + const opts = { + language: validLang, + ignoreIllegals: true, + theme: getCliHighlightTheme(theme), + }; + try { + return highlight(code, opts).split("\n"); + } catch { + // Same fallback as the markdown theme below, because both write into one + // memo: a differing fallback would make the result depend on which entry + // point happened to run first. + return code.split("\n").map((line) => theme.fg("mdCodeBlock", line)); + } + }); } /** @@ -1350,22 +1496,24 @@ export function getMarkdownTheme(activeTheme: Theme = theme): MarkdownTheme { highlightCode: (code: string, lang?: string): string[] => { // Validate language before highlighting to avoid stderr spam from cli-highlight const validLang = lang && supportsLanguage(lang) ? lang : undefined; - // Skip highlighting when no valid language is specified. cli-highlight's - // auto-detection is unreliable and can misidentify prose as AppleScript, - // LiveCodeServer, etc., coloring random English words as keywords. - if (!validLang) { - return code.split("\n").map((line) => activeTheme.fg("mdCodeBlock", line)); - } - const opts = { - language: validLang, - ignoreIllegals: true, - theme: getCliHighlightTheme(activeTheme), - }; - try { - return highlight(code, opts).split("\n"); - } catch { - return code.split("\n").map((line) => activeTheme.fg("mdCodeBlock", line)); - } + // No language means no highlighting at all, so the memo covers plain fences + // too. Both paths share one cache because the cache is keyed on the resolved + // theme instance, not on the closure that produced the lines. + return cachedHighlight(activeTheme, code, validLang ?? "", () => { + if (!validLang) { + return code.split("\n").map((line) => activeTheme.fg("mdCodeBlock", line)); + } + const opts = { + language: validLang, + ignoreIllegals: true, + theme: getCliHighlightTheme(activeTheme), + }; + try { + return highlight(code, opts).split("\n"); + } catch { + return code.split("\n").map((line) => activeTheme.fg("mdCodeBlock", line)); + } + }); }, }; } diff --git a/packages/coding-agent/src/utils/syntax-highlight.ts b/packages/coding-agent/src/utils/syntax-highlight.ts index 080c128c2..2514a703d 100644 --- a/packages/coding-agent/src/utils/syntax-highlight.ts +++ b/packages/coding-agent/src/utils/syntax-highlight.ts @@ -50,12 +50,30 @@ for (const [name, language] of Object.entries(eagerLanguages)) { let allLanguagesPromise: Promise | undefined; +/** + * Increments when the grammar set changes. + * + * `highlight.js` resolves embedded sub-languages through the same registry, so the + * same source and language can produce different spans before and after the + * remaining grammars load — a JavaScript template literal only gets HTML colouring + * once the XML grammar exists. Anything memoizing highlighted output has to include + * this in its key, because the language name alone does not identify the result. + */ +let grammarRevision = 0; + +export function highlightGrammarRevision(): number { + return grammarRevision; +} + export function loadAllHighlightLanguages(): Promise { if (!allLanguagesPromise) { allLanguagesPromise = new Promise((resolve) => { setImmediate(() => { void import("highlight.js/lib/index.js").then( - () => resolve(), + () => { + grammarRevision += 1; + resolve(); + }, () => { // Eager languages and plaintext fallback remain available. resolve(); diff --git a/packages/coding-agent/test/theme-highlight-cache.test.ts b/packages/coding-agent/test/theme-highlight-cache.test.ts new file mode 100644 index 000000000..7558ba4da --- /dev/null +++ b/packages/coding-agent/test/theme-highlight-cache.test.ts @@ -0,0 +1,232 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + clearHighlightCache, + getMarkdownTheme, + getThemeByName, + highlightCode, + initTheme, + setRegisteredThemes, +} from "../src/theme/theme.ts"; +import * as syntax from "../src/utils/syntax-highlight.ts"; + +/** + * Highlighted lines are memoized on (theme instance, language, source). + * + * The renderer treats a theme as a set of pure functions, so it cannot deduplicate + * this work itself: a fenced block is highlighted while it is being streamed, and + * any document that still renders whole re-highlights every finished block on every + * frame. That makes a stale or shared entry a visible colour bug rather than a + * slowdown, so the cases below are the ones where an entry could be served for the + * wrong inputs. + * + * Colours are asserted through the theme's own `syntaxKeyword`, so no ANSI literal + * is hard-coded here. + */ + +const CODE = "export function greet(name: string): string {\n return name;\n}"; + +/** Re-register the built-in themes a theme-switch case replaced. */ +function refreshRegisteredThemes(): void { + const builtin = ["dark", "light"] + .map((name) => getThemeByName(name)) + .filter((theme): theme is NonNullable => theme !== undefined); + setRegisteredThemes(builtin); +} + +function keywordColour(themeName: string): string { + const t = getThemeByName(themeName); + expect(t, `theme ${themeName} should be registered`).toBeDefined(); + return t!.fg("syntaxKeyword", "KEYWORD"); +} + +afterEach(() => { + initTheme("dark"); + // Every case starts from a cold memo, so none of them depends on another's keys. + clearHighlightCache(); + // The theme-switch case replaces the registry; leaving it replaced would make a + // later file's assertions depend on run order. + refreshRegisteredThemes(); +}); + +describe("highlight memo", () => { + it("serves a repeated call without re-highlighting", () => { + initTheme("dark"); + // Two requests, because the memo stores on the second miss (see below). + const first = highlightCode(CODE, "typescript"); + expect(first.length).toBeGreaterThan(1); + expect(highlightCode(CODE, "typescript")).toEqual(first); + + // Counting invocations is the only way to tell "memoized" from "deterministic". + const spy = vi.spyOn(syntax, "highlight"); + expect(highlightCode(CODE, "typescript")).toEqual(first); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("does not serve a cached unlabelled fence after the theme changes", () => { + // Unlabelled fences are restyled line by line from the theme, so they are the + // cheapest thing to memoize and the easiest to serve stale. + initTheme("dark"); + const dark = highlightCode("plain fence body", undefined).join("\n"); + + const themes = ["dark", "light"].map((name) => getThemeByName(name)).filter((t) => t !== undefined); + setRegisteredThemes(themes); + initTheme("light"); + + const light = highlightCode("plain fence body", undefined).join("\n"); + expect(light).not.toEqual(dark); + expect(light).toContain("plain fence body"); + }); + + it("does not serve a cached entry after the theme changes", () => { + initTheme("dark"); + const darkLines = highlightCode(CODE, "typescript"); + const darkKeyword = keywordColour("dark"); + + // Register both themes so the switch below is a real theme replacement. + const themes = ["dark", "light"].map((name) => getThemeByName(name)).filter((t) => t !== undefined); + setRegisteredThemes(themes); + initTheme("light"); + + const lightLines = highlightCode(CODE, "typescript"); + const lightKeyword = keywordColour("light"); + expect(lightKeyword).not.toEqual(darkKeyword); + expect(lightLines.join("\n")).not.toEqual(darkLines.join("\n")); + expect(lightLines.join("\n")).toContain("greet"); + }); + + it("keeps entries for an explicitly passed theme separate from the global one", () => { + initTheme("dark"); + const explicit = getThemeByName("light"); + expect(explicit).toBeDefined(); + + const explicitLines = getMarkdownTheme(explicit!).highlightCode!(CODE, "typescript"); + const globalLines = getMarkdownTheme().highlightCode!(CODE, "typescript"); + expect(explicitLines.join("\n")).not.toEqual(globalLines.join("\n")); + + // Asking again in the other order must not flip either result. + expect(getMarkdownTheme(explicit!).highlightCode!(CODE, "typescript")).toEqual(explicitLines); + expect(getMarkdownTheme().highlightCode!(CODE, "typescript")).toEqual(globalLines); + }); + + it("returns a private array so a mutating caller cannot corrupt the memo", () => { + initTheme("dark"); + // Snapshot by value: comparing against another call's return value would alias + // the memo entry and hide exactly the corruption this test exists to catch. + const expected = [...highlightCode(CODE, "typescript")]; + + // Mirrors core/tools/write.ts, which stores the result and then appends to it. + const mutated = highlightCode(CODE, "typescript"); + mutated[0] = "corrupted"; + mutated.push("extra"); + + expect([...highlightCode(CODE, "typescript")]).toEqual(expected); + }); + + it("keys on language as well as source", () => { + initTheme("dark"); + expect(highlightCode(CODE, "json")).not.toEqual(highlightCode(CODE, "typescript")); + }); + + it("does not spend the budget on keys that are seen only once", () => { + // A fence that is still being written yields a new key on every delta, and each + // prefix is dead as soon as the next one arrives. Retaining those would fill the + // budget with intermediate versions of one block and evict the finished blocks + // the memo exists to serve. + // + // Unlabelled fences keep this cheap: they are restyled line by line and never + // reach the highlighter, so the test measures retention rather than hljs. + initTheme("dark"); + const theme = getMarkdownTheme(); + // The block under test is labelled, so a miss is observable through the + // highlighter; the flood below is unlabelled, which is cheap and never calls it. + theme.highlightCode!(CODE, "typescript"); + theme.highlightCode!(CODE, "typescript"); + + // Enough distinct one-off sources to exceed the whole payload budget. + const line = "a plain fence row that is long enough to be worth measuring"; + const prefix = (index: number) => Array.from({ length: 500 }, (_, row) => `${index}-${row} ${line}`).join("\n"); + // ~6 MB of one-off payload, well past the whole budget. + for (let index = 0; index < 200; index++) theme.highlightCode!(prefix(index), undefined); + + const spy = vi.spyOn(syntax, "highlight"); + theme.highlightCode!(CODE, "typescript"); + expect(spy, "one-off keys consumed the budget and evicted a finished block").not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("recomputes after the grammar set changes", () => { + // `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 key without the grammar revision would freeze the pre-load + // result for a block that streams during startup. + initTheme("dark"); + const source = 'const t = html`
hello
`;'; + highlightCode(source, "javascript"); + highlightCode(source, "javascript"); + + const startRevision = syntax.highlightGrammarRevision(); + const spy = vi.spyOn(syntax, "highlight"); + const revision = vi.spyOn(syntax, "highlightGrammarRevision").mockReturnValue(startRevision + 1); + highlightCode(source, "javascript"); + revision.mockRestore(); + expect(spy, "the pre-load result was served after the grammar set changed").toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("re-highlights a huge source instead of retaining it", () => { + initTheme("dark"); + const huge = Array.from({ length: 8000 }, (_, index) => `const value${index} = ${index};`).join("\n"); + expect(huge.length).toBeGreaterThan(64 * 1024); + + // The memo needs two requests before it stores anything, so ask twice first. + highlightCode(huge, "typescript"); + highlightCode(huge, "typescript"); + + const spy = vi.spyOn(syntax, "highlight"); + expect(highlightCode(huge, "typescript").length).toBe(8000); + expect(spy, "a source above the size limit was retained").toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("evicts by payload budget rather than entry count", () => { + initTheme("dark"); + const blocks = Array.from({ length: 200 }, (_, index) => + Array.from({ length: 40 }, (_, row) => `const value${index}_${row} = ${row};`).join("\n"), + ); + // Two requests each, so every block is eligible for storage. + for (const block of blocks) highlightCode(block, "typescript"); + for (const block of blocks) highlightCode(block, "typescript"); + + // The earliest blocks must have been evicted to stay inside the budget, and + // recomputing them must still be correct. + const spy = vi.spyOn(syntax, "highlight"); + expect(highlightCode(blocks[0]!, "typescript").join("\n")).toContain("value0_0"); + expect(spy, "nothing was evicted, so the budget is not enforced").toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("uses the same fallback on both entry points", () => { + // One memo, two callers: if their `catch` branches differed, the stored value + // would depend on which one ran first. + initTheme("dark"); + const boom = vi.spyOn(syntax, "highlight").mockImplementation(() => { + throw new Error("highlighter exploded"); + }); + const viaModule = highlightCode(CODE, "typescript"); + const viaTheme = getMarkdownTheme().highlightCode!(CODE, "typescript"); + boom.mockRestore(); + expect(viaTheme).toEqual(viaModule); + }); + + it("agrees between the module entry point and the markdown theme", () => { + initTheme("dark"); + expect(getMarkdownTheme().highlightCode!(CODE, "typescript")).toEqual(highlightCode(CODE, "typescript")); + }); + + it("returns plain code when the language is unknown", () => { + initTheme("dark"); + const lines = highlightCode("not code at all", "definitely-not-a-language"); + expect(lines).toEqual([expect.stringContaining("not code at all")]); + }); +}); diff --git a/packages/tui/test/markdown-stream-prefix.test.ts b/packages/tui/test/markdown-stream-prefix.test.ts index 36efdb874..5831882e8 100644 --- a/packages/tui/test/markdown-stream-prefix.test.ts +++ b/packages/tui/test/markdown-stream-prefix.test.ts @@ -101,10 +101,9 @@ const HAZARDS: ReadonlyArray = [ ["paragraph-continuation", "intro\n\nfirst line\nsecond line\n\nthird\n"], ["image-line", "intro\n\n![alt](https://example.com/x.png)\n\ntail\n"], ["empty-first-block", "\n\nfirst\n\nsecond\n"], - // The block shapes below were contributed by the per-token cache investigation. - // They are not new constructs, but each one stresses a different part of the - // certification: link state that is balanced again, list numbering that starts off - // one, wide characters, and nested containers. + // Shapes that stress a different part of the certification: inline state that is + // balanced again, list numbering that starts off one, wide characters, and nested + // containers. [ "inline-anchor-balanced-then-autolink", 'open\n\nhttps://example.com\n\nclose \n\nhttps://example.com\n\nend\n',