From 0f21a3319d83d3a17176cf0850f69b3d9cc3f5ca Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sun, 1 Nov 2026 14:40:17 +0100 Subject: [PATCH] fix(coding-agent,tui): reuse syntax highlighting and rendered Markdown blocks Codemode and write rows keep their last highlighting in a CodeHighlighter that checks theme and grammar changes itself. Markdown reuses the top-level blocks of its last render, held weakly, and assistant messages reuse their Markdown instances while streaming. Text.setText() keeps its wrapped lines for unchanged text. closes #10971 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/core/tools/renderers/write.ts | 41 ++++-- .../src/extensions/codemode/renderer.ts | 14 +- .../components/assistant-message.ts | 85 +++++++---- .../components/markdown-transform.ts | 5 +- .../interactive/components/user-message.ts | 2 +- .../src/modes/interactive/theme/theme.ts | 64 ++++++--- .../coding-agent/src/utils/highlight-js.d.ts | 1 + .../src/utils/syntax-highlight.ts | 8 ++ .../test/assistant-message.test.ts | 34 ++++- .../test/code-highlighter.test.ts | 71 +++++++++ packages/tui/CHANGELOG.md | 2 + packages/tui/src/components/markdown.ts | 135 ++++++++++++------ packages/tui/src/components/text.ts | 2 + packages/tui/test/markdown.test.ts | 59 ++++++++ packages/tui/test/text.test.ts | 12 ++ 16 files changed, 420 insertions(+), 116 deletions(-) create mode 100644 packages/coding-agent/test/code-highlighter.test.ts create mode 100644 packages/tui/test/text.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ae5a6f324..d391a4f56 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -51,6 +51,7 @@ - Fixed `tool_search` and codemode `searchTools()` ranking a tool below tools that only mention its name in their descriptions, for example `peekaboo app` ranking `mcp__peekaboo__app` 14th. A tool matching more of the query's words now ranks above one matching fewer, a query word in a tool's name scores more than any number of mentions in its description, a tool whose whole name is in the query gets a large bonus, and long server instructions no longer lower the scores of that server's tools ([#11331](https://github.com/earendil-works/pi/issues/11331)) - Fixed `tool_search` and codemode `searchTools()` finding nothing for Chinese, Japanese, Thai, and other non-ASCII queries, because the tokenizer kept only ASCII letters and digits. Words in any script now match, text without spaces between words such as `搜索记录` is split into words, and canonically equivalent text such as `café` and `cafe\u0301` matches ([#10816](https://github.com/earendil-works/pi/issues/10816), [#11037](https://github.com/earendil-works/pi/pull/11037) by [@haoqixu](https://github.com/haoqixu)) - Fixed `pi update` stopping with `git clean -fdx failed with code 1` on Windows when another pi session had a git extension's native module or executable open. Windows cannot delete such files, so the update left the new code with the old dependencies. pi now warns, installs the dependencies anyway, and retries the clean on the next update ([#11597](https://github.com/earendil-works/pi/issues/11597)) +- Fixed tool rows and streamed answers highlighting the same code again on every redraw. A running codemode row highlighted its whole script on every progress update and timer tick, a complete `write` row its whole file on every update and expand toggle, a streamed `write` its first 50 lines on every delta, and a streamed answer every earlier code block on every delta. Rows now reuse their last highlighting until the code, theme, or loaded grammars change, and assistant messages keep their rendered Markdown blocks while streaming ([#10971](https://github.com/earendil-works/pi/issues/10971)) ### Removed diff --git a/packages/coding-agent/src/core/tools/renderers/write.ts b/packages/coding-agent/src/core/tools/renderers/write.ts index 7c8f32636..8e2338a7d 100644 --- a/packages/coding-agent/src/core/tools/renderers/write.ts +++ b/packages/coding-agent/src/core/tools/renderers/write.ts @@ -8,7 +8,12 @@ import { Container, Text } from "@earendil-works/pi-tui"; import { keyHint } from "../../../modes/interactive/components/keybinding-hints.ts"; -import { getLanguageFromPath, highlightCode, type Theme } from "../../../modes/interactive/theme/theme.ts"; +import { + CodeHighlighter, + getLanguageFromPath, + highlightCode, + type Theme, +} from "../../../modes/interactive/theme/theme.ts"; import type { ToolDefinition, ToolRenderResultOptions } from "../../extensions/types.ts"; import { normalizeDisplayText, renderToolPath, replaceTabs, str } from "../render-utils.ts"; @@ -20,7 +25,12 @@ type WriteHighlightCache = { highlightedLines: string[]; }; class WriteCallRenderComponent extends Text { + // Line by line highlighting of a call whose content is still streaming. cache?: WriteHighlightCache; + // A streamed call highlights its first lines on every delta, which stop changing once the file is longer, and a + // complete call highlights the whole file on every update and expand toggle. Either way the row reuses the last + // highlighting; it also holds the only copy of a complete call's displayed source. + readonly highlighter = new CodeHighlighter(); constructor() { super("", 0, 0); @@ -31,11 +41,11 @@ function highlightSingleLine(line: string, lang: string): string { const highlighted = highlightCode(line, lang); return highlighted[0] ?? ""; } -function refreshWriteHighlightPrefix(cache: WriteHighlightCache): void { +function refreshWriteHighlightPrefix(cache: WriteHighlightCache, highlighter: CodeHighlighter): void { const prefixCount = Math.min(WRITE_PARTIAL_FULL_HIGHLIGHT_LINES, cache.normalizedLines.length); if (prefixCount === 0) return; const prefixSource = cache.normalizedLines.slice(0, prefixCount).join("\n"); - const prefixHighlighted = highlightCode(prefixSource, cache.lang); + const prefixHighlighted = highlighter.highlight(prefixSource, cache.lang); for (let i = 0; i < prefixCount; i++) { cache.highlightedLines[i] = prefixHighlighted[i] ?? highlightSingleLine(cache.normalizedLines[i] ?? "", cache.lang); @@ -58,12 +68,14 @@ function updateWriteHighlightCacheIncremental( cache: WriteHighlightCache | undefined, rawPath: string | null, fileContent: string, + highlighter: CodeHighlighter, ): WriteHighlightCache | undefined { const lang = rawPath ? getLanguageFromPath(rawPath) : undefined; if (!lang) return undefined; if (!cache) return rebuildWriteHighlightCacheFull(rawPath, fileContent); - if (cache.lang !== lang || cache.rawPath !== rawPath) return rebuildWriteHighlightCacheFull(rawPath, fileContent); - if (!fileContent.startsWith(cache.rawContent)) return rebuildWriteHighlightCacheFull(rawPath, fileContent); + if (cache.lang !== lang || cache.rawPath !== rawPath || !fileContent.startsWith(cache.rawContent)) { + return rebuildWriteHighlightCacheFull(rawPath, fileContent); + } if (fileContent.length === cache.rawContent.length) return cache; const deltaRaw = fileContent.slice(cache.rawContent.length); @@ -83,10 +95,10 @@ function updateWriteHighlightCacheIncremental( cache.normalizedLines.push(segments[i]); cache.highlightedLines.push(highlightSingleLine(segments[i], cache.lang)); } - refreshWriteHighlightPrefix(cache); + refreshWriteHighlightPrefix(cache, highlighter); return cache; } -function trimTrailingEmptyLines(lines: string[]): string[] { +function trimTrailingEmptyLines(lines: readonly string[]): string[] { let end = lines.length; while (end > 0 && lines[end - 1] === "") { end--; @@ -98,6 +110,7 @@ function formatWriteCall( options: ToolRenderResultOptions, theme: Theme, cache: WriteHighlightCache | undefined, + highlighter: CodeHighlighter, cwd: string, ): string { const rawPath = str(args?.file_path ?? args?.path); @@ -110,7 +123,7 @@ function formatWriteCall( } else if (fileContent) { const lang = rawPath ? getLanguageFromPath(rawPath) : undefined; const renderedLines = lang - ? (cache?.highlightedLines ?? highlightCode(replaceTabs(normalizeDisplayText(fileContent)), lang)) + ? (cache?.highlightedLines ?? highlighter.highlight(replaceTabs(normalizeDisplayText(fileContent)), lang)) : normalizeDisplayText(fileContent).split("\n"); const lines = trimTrailingEmptyLines(renderedLines); const totalLines = lines.length; @@ -149,19 +162,17 @@ export const writeRenderers: Pick, "renderCall" | "rend const fileContent = str(renderArgs?.content); const component = (context.lastComponent as WriteCallRenderComponent | undefined) ?? new WriteCallRenderComponent(); - if (fileContent !== null) { - component.cache = context.argsComplete - ? rebuildWriteHighlightCacheFull(rawPath, fileContent) - : updateWriteHighlightCacheIncremental(component.cache, rawPath, fileContent); - } else { - component.cache = undefined; - } + component.cache = + fileContent !== null && !context.argsComplete + ? updateWriteHighlightCacheIncremental(component.cache, rawPath, fileContent, component.highlighter) + : undefined; component.setText( formatWriteCall( renderArgs, { expanded: context.expanded, isPartial: context.isPartial }, theme, component.cache, + component.highlighter, context.cwd, ), ); diff --git a/packages/coding-agent/src/extensions/codemode/renderer.ts b/packages/coding-agent/src/extensions/codemode/renderer.ts index 8d2937351..660803e68 100644 --- a/packages/coding-agent/src/extensions/codemode/renderer.ts +++ b/packages/coding-agent/src/extensions/codemode/renderer.ts @@ -12,7 +12,7 @@ import type { ToolDefinition } from "../../core/extensions/types.ts"; import { getTextOutput, replaceTabs, startToolTimer, str, toolTimerText } from "../../core/tools/render-utils.ts"; import { keyHint } from "../../modes/interactive/components/keybinding-hints.ts"; import { VisualLinePreview } from "../../modes/interactive/components/visual-truncate.ts"; -import { highlightCode, type Theme } from "../../modes/interactive/theme/theme.ts"; +import { CodeHighlighter, type Theme } from "../../modes/interactive/theme/theme.ts"; import type { CodemodeNestedCall, CodemodeToolDetails } from "./tool.ts"; const CODE_PREVIEW_LINES = 10; @@ -21,6 +21,12 @@ const OUTPUT_PREVIEW_LINES = 5; const COLLAPSED_ARGS_CHARS = 80; const SCRIPT_HEADER = /^Script (completed|failed)\nWall time [\d.]+ seconds\nOutput:\n$/; +/** Renderer state of a codemode row. */ +interface CodemodeRenderState { + // Progress, timer ticks and expanding redraw the call, so the row keeps the script's highlighting. + scriptHighlighter?: CodeHighlighter; +} + function expandHint(theme: Theme, hidden: number, noun: string): string { return `${theme.fg("muted", `... (${hidden} more ${noun},`)} ${keyHint("app.tools.expand", "to expand")}${theme.fg("muted", ")")}`; } @@ -79,7 +85,11 @@ export const codemodeRenderers: Pick< } component.addChild(new Text(title, 0, 0)); if (code) { - const highlighted = highlightCode(replaceTabs(code.replace(/\r/g, "").trimEnd()), "javascript").join("\n"); + const state = context.state as CodemodeRenderState; + state.scriptHighlighter ??= new CodeHighlighter(); + const highlighted = state.scriptHighlighter + .highlight(replaceTabs(code.replace(/\r/g, "").trimEnd()), "javascript") + .join("\n"); component.addChild( context.expanded ? new Text(highlighted, 0, 0) diff --git a/packages/coding-agent/src/modes/interactive/components/assistant-message.ts b/packages/coding-agent/src/modes/interactive/components/assistant-message.ts index 67bb71a3f..7cb296031 100644 --- a/packages/coding-agent/src/modes/interactive/components/assistant-message.ts +++ b/packages/coding-agent/src/modes/interactive/components/assistant-message.ts @@ -16,6 +16,15 @@ const OSC133_ZONE_START = "\x1b]133;A\x07"; const OSC133_ZONE_END = "\x1b]133;B\x07"; const OSC133_ZONE_FINAL = "\x1b]133;C\x07"; +/** + * A Markdown block of the last update. The next update reuses it at the same position for the same kind of content, so + * a streamed answer keeps the blocks it already rendered, also once the stream ends. + */ +interface MarkdownSlot { + kind: "text" | "thinking"; + markdown: Markdown; +} + /** * Component that renders a complete assistant message */ @@ -32,6 +41,7 @@ export class AssistantMessageComponent extends Container { private messageId?: string; private timestamp?: string; private thinkingVisibilityOverrides = new Map(); + private markdownSlots: MarkdownSlot[] = []; constructor( message?: AssistantMessage, @@ -86,6 +96,8 @@ export class AssistantMessageComponent extends Container { setOutputPad(padding: number): void { this.outputPad = padding; + // Markdown takes its padding at construction. + this.markdownSlots = []; if (this.lastMessage) { this.updateContent(this.lastMessage); } @@ -108,6 +120,15 @@ export class AssistantMessageComponent extends Container { // Clear content container this.contentContainer.clear(); + const previousSlots = this.markdownSlots; + this.markdownSlots = []; + const markdownBlock = (kind: MarkdownSlot["kind"], text: string, create: () => Markdown): Markdown => { + const slot = previousSlots[this.markdownSlots.length]; + const markdown = slot?.kind === kind ? slot.markdown : create(); + markdown.setText(text); + this.markdownSlots.push({ kind, markdown }); + return markdown; + }; // Spacers are added lazily, right before the next visible content block. This puts a blank // line before the first visible block and after each visible thinking run that is followed @@ -128,16 +149,22 @@ export class AssistantMessageComponent extends Container { if (content.type === "text" && content.text.trim()) { // Assistant text messages with no background - trim the text // Set paddingY=0 to avoid extra spacing before tool executions + const text = content.text.trim(); addVisibleChild( - new Markdown(content.text.trim(), this.outputPad, 0, this.markdownTheme, undefined, { - transform: createMarkdownTransform( - "assistant", - this.isStreaming, - this.markdownTransformers, - this.messageId, - this.timestamp, - ), - }), + markdownBlock( + "text", + text, + () => + new Markdown(text, this.outputPad, 0, this.markdownTheme, undefined, { + transform: createMarkdownTransform( + "assistant", + () => this.isStreaming, + this.markdownTransformers, + this.messageId, + this.timestamp, + ), + }), + ), ); } else if (content.type === "thinking") { const thinkingBlocks: string[] = []; @@ -163,26 +190,32 @@ export class AssistantMessageComponent extends Container { if (hidden && !this.hiddenThinkingLabel.trim()) { continue; } + const thinking = thinkingBlocks.join("\n\n"); const thinkingComponent = hidden ? new Text(theme.italic(theme.fg("thinkingText", this.hiddenThinkingLabel)), this.outputPad, 0) - : new Markdown( - thinkingBlocks.join("\n\n"), - this.outputPad, - 0, - this.markdownTheme, - { - color: (text: string) => theme.fg("thinkingText", text), - italic: true, - }, - { - transform: createMarkdownTransform( - "assistant-thinking", - this.isStreaming, - this.markdownTransformers, - this.messageId, - this.timestamp, + : markdownBlock( + "thinking", + thinking, + () => + new Markdown( + thinking, + this.outputPad, + 0, + this.markdownTheme, + { + color: (text: string) => theme.fg("thinkingText", text), + italic: true, + }, + { + transform: createMarkdownTransform( + "assistant-thinking", + () => this.isStreaming, + this.markdownTransformers, + this.messageId, + this.timestamp, + ), + }, ), - }, ); addVisibleChild( new MouseRegion(thinkingComponent, (event) => { diff --git a/packages/coding-agent/src/modes/interactive/components/markdown-transform.ts b/packages/coding-agent/src/modes/interactive/components/markdown-transform.ts index 4ff7685c2..58bae9094 100644 --- a/packages/coding-agent/src/modes/interactive/components/markdown-transform.ts +++ b/packages/coding-agent/src/modes/interactive/components/markdown-transform.ts @@ -1,8 +1,9 @@ import type { MarkdownTransformContext, MarkdownTransformer } from "../../../core/extensions/types.ts"; +/** Applies extension Markdown transformers. `isStreaming` is read per transform, so a reused component sees a stream end. */ export function createMarkdownTransform( messageType: MarkdownTransformContext["messageType"], - isStreaming: boolean, + isStreaming: () => boolean, transformers: readonly MarkdownTransformer[], messageId?: string, timestamp?: string, @@ -10,7 +11,7 @@ export function createMarkdownTransform( return (markdown, availableWidth) => applyMarkdownTransformers( markdown, - { messageType, isStreaming, availableWidth, messageId, timestamp }, + { messageType, isStreaming: isStreaming(), availableWidth, messageId, timestamp }, transformers, ); } diff --git a/packages/coding-agent/src/modes/interactive/components/user-message.ts b/packages/coding-agent/src/modes/interactive/components/user-message.ts index d8ca373cc..e313e0242 100644 --- a/packages/coding-agent/src/modes/interactive/components/user-message.ts +++ b/packages/coding-agent/src/modes/interactive/components/user-message.ts @@ -60,7 +60,7 @@ export class UserMessageComponent extends Container { preserveBackslashEscapes: true, transform: createMarkdownTransform( "user", - false, + () => false, this.markdownTransformers, this.messageId, this.timestamp, diff --git a/packages/coding-agent/src/modes/interactive/theme/theme.ts b/packages/coding-agent/src/modes/interactive/theme/theme.ts index 876ba9e86..922b23d9f 100644 --- a/packages/coding-agent/src/modes/interactive/theme/theme.ts +++ b/packages/coding-agent/src/modes/interactive/theme/theme.ts @@ -26,7 +26,7 @@ import chalk from "chalk"; 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 { getHighlightLanguageCount, highlight, supportsLanguage } from "../../../utils/syntax-highlight.ts"; import { stripBom } from "../../../utils/text.ts"; import { generateSystemThemeColors, SYSTEM_THEME_NAME, terminalAppearance } from "./system-theme.ts"; import { @@ -658,6 +658,11 @@ export const theme: Theme = new Proxy({} as Theme, { }, }); +/** The theme instance behind the `theme` proxy, whose own identity never changes. */ +function getGlobalTheme(): Theme | undefined { + return (globalThis as Record)[THEME_KEY]; +} + function setGlobalTheme(t: Theme): void { (globalThis as Record)[THEME_KEY] = t; (globalThis as Record)[THEME_KEY_OLD] = t; @@ -944,7 +949,41 @@ export function highlightCode(code: string, lang?: string): string[] { try { return highlight(code, opts).split("\n"); } catch { - return code.split("\n"); + return code.split("\n").map((line) => theme.fg("mdCodeBlock", line)); + } +} + +/** + * Highlights code for one owner, such as a tool row, and reuses the last result while the code, the language, the + * theme and the registered grammars stay the same. Each owner keeps its own highlighter, so cached results live as long + * as the code they show instead of in a process-wide cache. + */ +export class CodeHighlighter { + private last?: { + code: string; + lang: string | undefined; + theme: Theme | undefined; + languageCount: number; + lines: readonly string[]; + }; + + /** Like {@link highlightCode}. The result is shared with later calls, so callers must not modify it. */ + highlight(code: string, lang: string | undefined): readonly string[] { + // Renderers cannot tell a theme change from any other redraw, so the highlighter checks what its result depends on. + const currentTheme = getGlobalTheme(); + const languageCount = getHighlightLanguageCount(); + const last = this.last; + if ( + last?.code === code && + last.lang === lang && + last.theme === currentTheme && + last.languageCount === languageCount + ) { + return last.lines; + } + const lines = highlightCode(code, lang); + this.last = { code, lang, theme: currentTheme, languageCount, lines }; + return lines; } } @@ -1035,26 +1074,7 @@ export function getMarkdownTheme(): MarkdownTheme { italic: (text: string) => theme.italic(text), underline: (text: string) => theme.underline(text), strikethrough: (text: string) => theme.strikethrough(text), - 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").map((line) => theme.fg("mdCodeBlock", line)); - } - }, + highlightCode, }; } diff --git a/packages/coding-agent/src/utils/highlight-js.d.ts b/packages/coding-agent/src/utils/highlight-js.d.ts index d4b71174e..c4c951fce 100644 --- a/packages/coding-agent/src/utils/highlight-js.d.ts +++ b/packages/coding-agent/src/utils/highlight-js.d.ts @@ -17,6 +17,7 @@ interface HighlightJsApi { highlight(code: string, options: HighlightJsOptions): HighlightJsResult; highlightAuto(code: string, languageSubset?: string[]): HighlightJsResult; getLanguage(name: string): HighlightJsLanguageDefinition | undefined; + listLanguages(): string[]; registerLanguage(name: string, language: HighlightJsLanguageFactory): void; } diff --git a/packages/coding-agent/src/utils/syntax-highlight.ts b/packages/coding-agent/src/utils/syntax-highlight.ts index 1ee1986b9..91375bb01 100644 --- a/packages/coding-agent/src/utils/syntax-highlight.ts +++ b/packages/coding-agent/src/utils/syntax-highlight.ts @@ -217,3 +217,11 @@ export function highlight(code: string, options: HighlightOptions = {}): string export function supportsLanguage(name: string): boolean { return hljs.getLanguage(name) !== undefined; } + +/** + * Number of registered grammars. It grows when the deferred grammars load, which can change the highlighting of + * code that embeds another language, such as HTML in a JavaScript template literal. + */ +export function getHighlightLanguageCount(): number { + return hljs.listLanguages().length; +} diff --git a/packages/coding-agent/test/assistant-message.test.ts b/packages/coding-agent/test/assistant-message.test.ts index 42f6e496d..2cf368915 100644 --- a/packages/coding-agent/test/assistant-message.test.ts +++ b/packages/coding-agent/test/assistant-message.test.ts @@ -1,10 +1,11 @@ import type { AssistantMessage } from "@earendil-works/pi-ai"; import type { TuiMouseEvent } from "@earendil-works/pi-tui"; -import { describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; import { AssistantMessageComponent } from "../src/modes/interactive/components/assistant-message.ts"; import { UserMessageComponent } from "../src/modes/interactive/components/user-message.ts"; import { initTheme } from "../src/modes/interactive/theme/theme.ts"; import { stripAnsi } from "../src/utils/ansi.ts"; +import * as syntaxHighlight from "../src/utils/syntax-highlight.ts"; const OSC133_ZONE_START = "\x1b]133;A\x07"; const OSC133_ZONE_END = "\x1b]133;B\x07"; @@ -34,6 +35,37 @@ function createAssistantMessage( } describe("AssistantMessageComponent", () => { + afterEach(() => vi.restoreAllMocks()); + + test("reuses its Markdown across streaming and its end, and renders like a new component after a theme change", () => { + initTheme("dark"); + const highlight = vi.spyOn(syntaxHighlight, "highlight"); + const streaming: boolean[] = []; + const code = "```ts\nconst a = 1;\n```"; + const content = (text: string): AssistantMessage["content"] => [ + { type: "thinking", thinking: "Thinking with **bold** text" }, + { type: "text", text }, + ]; + const component = new AssistantMessageComponent(undefined, false, undefined, "Thinking...", 1, [ + (markdown, { isStreaming }) => { + streaming.push(isStreaming); + return markdown; + }, + ]); + component.updateContent(createAssistantMessage(content(code)), true); + component.render(80); + const message = createAssistantMessage(content(`${code}\n\nThe end.`)); + component.updateContent(message, false); + component.render(80); + expect(highlight).toHaveBeenCalledTimes(1); + expect(streaming.at(-1)).toBe(false); + + initTheme("light"); + component.invalidate(); + expect(component.render(80)).toEqual(new AssistantMessageComponent(message).render(80)); + initTheme("dark"); + }); + test("adds OSC 133 zone markers to assistant messages without tool calls", () => { initTheme("dark"); diff --git a/packages/coding-agent/test/code-highlighter.test.ts b/packages/coding-agent/test/code-highlighter.test.ts new file mode 100644 index 000000000..b0d0433b4 --- /dev/null +++ b/packages/coding-agent/test/code-highlighter.test.ts @@ -0,0 +1,71 @@ +import type { TUI } from "@earendil-works/pi-tui"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { writeRenderers } from "../src/core/tools/renderers/write.ts"; +import { codemodeRenderers } from "../src/extensions/codemode/renderer.ts"; +import { ToolExecutionComponent } from "../src/modes/interactive/components/tool-execution.ts"; +import { CodeHighlighter, highlightCode, initTheme } from "../src/modes/interactive/theme/theme.ts"; +import * as syntaxHighlight from "../src/utils/syntax-highlight.ts"; + +const tui = { requestRender: () => {} } as unknown as TUI; + +describe("CodeHighlighter", () => { + beforeEach(() => initTheme("dark")); + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("reuses its last result until the code, the theme or the grammars change", async () => { + const code = 'const page = html`
ok
`;'; + const highlighter = new CodeHighlighter(); + const first = highlighter.highlight(code, "javascript"); + expect(highlighter.highlight(code, "javascript")).toBe(first); + expect(highlighter.highlight("const a = 1;", "javascript")).not.toBe(first); + + initTheme("light"); + expect(highlighter.highlight(code, "javascript")).toEqual(highlightCode(code, "javascript")); + const light = highlighter.highlight(code, "javascript"); + await syntaxHighlight.loadAllHighlightLanguages(); + expect(highlighter.highlight(code, "javascript")).not.toEqual(light); + }); + + it("codemode highlights the script once across progress, timer ticks, expanding and completion", () => { + vi.useFakeTimers(); + const highlight = vi.spyOn(syntaxHighlight, "highlight"); + const row = new ToolExecutionComponent( + "codemode", + "c", + { code: "await tools.read({});" }, + {}, + codemodeRenderers, + tui, + "/", + ); + row.setArgsComplete(); + row.markExecutionStarted(); + row.updateResult({ content: [], details: { calls: [] }, isError: false }, true); + vi.advanceTimersByTime(2_000); + row.setExpanded(true); + row.updateResult({ content: [{ type: "text", text: "done" }], details: { calls: [] }, isError: false }, false); + row.render(80); + expect(highlight).toHaveBeenCalledTimes(1); + }); + + it("write highlights unchanged first lines and a complete file only once", () => { + const content = Array.from({ length: 60 }, (_, i) => `const v${i} = ${i};`).join("\n"); + const row = new ToolExecutionComponent("write", "w", { path: "a.ts", content }, {}, writeRenderers, tui, "/"); + row.updateArgs({ path: "a.ts", content: `${content} //` }); + row.render(80); + const highlight = vi.spyOn(syntaxHighlight, "highlight"); + // Only the changed last line is highlighted again. + row.updateArgs({ path: "a.ts", content: `${content} // more` }); + row.render(80); + expect(highlight).toHaveBeenCalledTimes(1); + + row.setArgsComplete(); + row.setExpanded(true); + row.setExpanded(false); + row.render(80); + expect(highlight).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index d03c3cfc8..1b13cf0a7 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -18,6 +18,8 @@ - Fixed display math wider than the content width being wrapped row by row, which split fractions and sums apart. It now falls back to the raw LaTeX source ([#11483](https://github.com/earendil-works/pi/issues/11483)) - Fixed `tui.select.pageUp` and `tui.select.pageDown` being ignored by `SelectList`, `SettingsList`, and editor autocomplete ([#7629](https://github.com/earendil-works/pi/issues/7629), based on [#7680](https://github.com/earendil-works/pi/pull/7680) by [@muyiyr](https://github.com/muyiyr)) - Fixed `wrapTextWithAnsi()` starting the next line with a space when a styled word ended exactly at the wrap width, and strikethrough continuing into the right padding of wrapped lines ([#10925](https://github.com/earendil-works/pi/issues/10925)) +- Fixed `Markdown` styling, highlighting, and wrapping every block on each render. It now reuses the top-level blocks of its last render, so a streaming message renders only its changed last block, and a resize reuses highlighted code. Reused blocks are held weakly and dropped by `invalidate()`, which a theme change must call because `MarkdownTheme.highlightCode` results are reused until then ([#10971](https://github.com/earendil-works/pi/issues/10971)) +- Fixed `Text.setText()` with unchanged text dropping the wrapped lines, so renderers that set the same text on every update wrapped it again ([#10971](https://github.com/earendil-works/pi/issues/10971)) ## [1.4.0] - 2026-10-21 diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index bacfe29f8..a21f75181 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -1,4 +1,4 @@ -import { Marked, type Token, Tokenizer, type TokenizerExtension, type Tokens } from "marked"; +import { Marked, type Token, Tokenizer, type TokenizerExtension, type Tokens, type TokensList } from "marked"; import markedCjkFriendly from "marked-cjk-friendly"; import { renderLatex } from "../latex.ts"; import { getCapabilities, hyperlink, isImageLine } from "../terminal-image.ts"; @@ -244,6 +244,7 @@ export interface MarkdownTheme { italic: (text: string) => string; strikethrough: (text: string) => string; underline: (text: string) => string; + /** Highlighted lines of a code block. A Markdown component reuses the result for the same code until invalidate(). */ highlightCode?: (code: string, lang?: string) => string[]; /** Prefix applied to each rendered code block line (default: " ") */ codeBlockIndent?: string; @@ -265,6 +266,16 @@ interface InlineStyleContext { stylePrefix: string; } +/** A top-level block of the last render, keyed by its source, reused by the next render. */ +interface RenderedBlock { + /** Width and type of the following block (which decides the spacing) that the lines were rendered for. */ + width: number; + nextTokenType: string | undefined; + lines: string[]; + /** Highlighted lines of the block's code, keyed by language and source; they survive a resize or new spacing. */ + highlightedCode: Map; +} + export class Markdown implements Component { private text: string; private paddingX: number; // Left/right padding @@ -281,7 +292,14 @@ export class Markdown implements Component { // Parsed tokens depend only on the source, so they survive theme and width invalidation. Held weakly: a token tree is // about ten times the size of its source, and every message of a long transcript keeps a Markdown component. The // tokens survive a burst of re-renders, such as a theme preview, and are collected afterwards. - private cachedTokens?: WeakRef<{ source: string; tokens: Token[] }>; + private cachedTokens?: WeakRef<{ source: string; tokens: TokensList }>; + // The last render's top-level blocks. While a message streams only its last block changes, so a render styles and + // wraps only that block. Each render keeps only the blocks it used, which drops a streamed block's earlier versions. + // Held weakly like the tokens: the blocks repeat the source in their keys and the code in their highlighting, and + // only a burst of renders, such as streaming or a resize, needs them. + private blocks?: WeakRef>; + // Highlighted code of the block being rendered, only set while renderBlock runs. + private blockHighlightedCode?: Map; constructor( text: string, @@ -301,10 +319,17 @@ export class Markdown implements Component { setText(text: string): void { this.text = text; - this.invalidate(); + this.clearRenderCache(); } invalidate(): void { + this.clearRenderCache(); + // The theme may have changed, which changes every styled line and the default style's prefix. + this.blocks = undefined; + this.defaultStylePrefix = undefined; + } + + private clearRenderCache(): void { this.cachedText = undefined; this.cachedWidth = undefined; this.cachedLines = undefined; @@ -322,6 +347,7 @@ export class Markdown implements Component { // Don't render anything if there's no actual text if (!text || text.trim() === "") { + this.blocks = undefined; const result: string[] = []; // Update cache this.cachedText = this.text; @@ -342,65 +368,39 @@ export class Markdown implements Component { this.cachedTokens = new WeakRef({ source: normalizedText, tokens }); } - // Convert tokens to styled terminal output - const renderedLines: string[] = []; - + // A reference link resolves through a definition anywhere in the text, so blocks with the same source can render + // differently; such texts are rare in practice and render without reusing blocks. + const reuseBlocks = Object.keys(tokens.links).length === 0; + const previousBlocks = this.blocks?.deref(); + const blocks = new Map(); + this.blocks = new WeakRef(blocks); + const contentLines: string[] = []; for (let i = 0; i < tokens.length; i++) { const token = tokens[i]; - const nextToken = tokens[i + 1]; - const tokenLines = this.renderToken(token, contentWidth, nextToken?.type); - for (const tokenLine of tokenLines) { - renderedLines.push(tokenLine); + const nextTokenType = tokens[i + 1]?.type; + const reused = reuseBlocks ? (blocks.get(token.raw) ?? previousBlocks?.get(token.raw)) : undefined; + const block = reused ?? { width: -1, nextTokenType, lines: [], highlightedCode: new Map() }; + if (block.width !== width || block.nextTokenType !== nextTokenType) { + this.renderBlock(block, token, nextTokenType, width, contentWidth); } - } - - // Wrap lines (NO padding, NO background yet) - const wrappedLines: string[] = []; - for (const line of renderedLines) { - if (isImageLine(line)) { - wrappedLines.push(line); - } else { - for (const wrappedLine of wrapTextWithAnsi(line, contentWidth)) { - wrappedLines.push(wrappedLine); - } - } - } - - // Add margins and background to each wrapped line - const leftMargin = " ".repeat(this.paddingX); - const rightMargin = " ".repeat(this.paddingX); - const bgFn = this.defaultTextStyle?.bgColor; - const contentLines: string[] = []; - - for (const line of wrappedLines) { - if (isImageLine(line)) { + if (reuseBlocks) blocks.set(token.raw, block); + for (const line of block.lines) { contentLines.push(line); - continue; - } - - const lineWithMargins = leftMargin + line + rightMargin; - - if (bgFn) { - contentLines.push(applyBackgroundToLine(lineWithMargins, width, bgFn)); - } else { - // No background - just pad to width - const visibleLen = visibleWidth(lineWithMargins); - const paddingNeeded = Math.max(0, width - visibleLen); - contentLines.push(lineWithMargins + " ".repeat(paddingNeeded)); } } // Add top/bottom padding (empty lines) + const bgFn = this.defaultTextStyle?.bgColor; const emptyLine = " ".repeat(width); const emptyLines: string[] = []; for (let i = 0; i < this.paddingY; i++) { const line = bgFn ? applyBackgroundToLine(emptyLine, width, bgFn) : emptyLine; emptyLines.push(line); } + flattenLines(emptyLines); // Combine top padding, content, and bottom padding const result = emptyLines.concat(contentLines, emptyLines); - flattenLines(result); // Update cache this.cachedText = this.text; @@ -410,6 +410,42 @@ export class Markdown implements Component { return result.length > 0 ? result : [""]; } + /** Render one top-level block to output lines: wrapped, with margins, and padded or backgrounded to the width. */ + private renderBlock( + block: RenderedBlock, + token: Token, + nextTokenType: string | undefined, + width: number, + contentWidth: number, + ): void { + this.blockHighlightedCode = block.highlightedCode; + const leftMargin = " ".repeat(this.paddingX); + const rightMargin = " ".repeat(this.paddingX); + const bgFn = this.defaultTextStyle?.bgColor; + const lines: string[] = []; + for (const line of this.renderToken(token, contentWidth, nextTokenType)) { + if (isImageLine(line)) { + lines.push(line); + continue; + } + for (const wrappedLine of wrapTextWithAnsi(line, contentWidth)) { + const lineWithMargins = leftMargin + wrappedLine + rightMargin; + if (bgFn) { + lines.push(applyBackgroundToLine(lineWithMargins, width, bgFn)); + } else { + // No background - just pad to width + const paddingNeeded = Math.max(0, width - visibleWidth(lineWithMargins)); + lines.push(lineWithMargins + " ".repeat(paddingNeeded)); + } + } + } + this.blockHighlightedCode = undefined; + flattenLines(lines); + block.lines = lines; + block.width = width; + block.nextTokenType = nextTokenType; + } + /** * Apply default text style to a string. * This is the base styling applied to all text content. @@ -567,8 +603,13 @@ export class Markdown implements Component { const indent = this.theme.codeBlockIndent ?? " "; lines.push(this.theme.codeBlockBorder(`\`\`\`${token.lang || ""}`)); if (this.theme.highlightCode) { - const highlightedLines = this.theme.highlightCode(token.text, token.lang); - for (const hlLine of highlightedLines) { + const key = `${token.lang ?? ""}\0${token.text}`; + let highlighted = this.blockHighlightedCode?.get(key); + if (!highlighted) { + highlighted = this.theme.highlightCode(token.text, token.lang); + this.blockHighlightedCode?.set(key, highlighted); + } + for (const hlLine of highlighted) { lines.push(`${indent}${hlLine}`); } } else { diff --git a/packages/tui/src/components/text.ts b/packages/tui/src/components/text.ts index 024748912..c9aea3d13 100644 --- a/packages/tui/src/components/text.ts +++ b/packages/tui/src/components/text.ts @@ -23,6 +23,8 @@ export class Text implements Component { } setText(text: string): void { + // Renderers often set the same text on every update; keep the wrapped lines then. + if (text === this.text) return; this.text = text; this.cachedText = undefined; this.cachedWidth = undefined; diff --git a/packages/tui/test/markdown.test.ts b/packages/tui/test/markdown.test.ts index c1615df66..e0f5b4dd5 100644 --- a/packages/tui/test/markdown.test.ts +++ b/packages/tui/test/markdown.test.ts @@ -1,5 +1,7 @@ import assert from "node:assert"; import { afterEach, describe, it } from "node:test"; +import { setFlagsFromString } from "node:v8"; +import { runInNewContext } from "node:vm"; import type { Terminal as XtermTerminalType } from "@xterm/headless"; import { Chalk } from "chalk"; import { Markdown, type MarkdownTheme } from "../src/components/markdown.ts"; @@ -65,6 +67,63 @@ describe("Markdown component", () => { }); }); + describe("Block reuse", () => { + afterEach(() => resetCapabilitiesCache()); + + function countingTheme(calls: string[]): MarkdownTheme { + return { + ...defaultMarkdownTheme, + highlightCode: (code, lang) => { + calls.push(`${lang}:${code}`); + return code.split("\n").map((line) => chalk.green(line)); + }, + }; + } + + it("highlights a code block once across streamed text and resizes, and again after invalidate", () => { + const calls: string[] = []; + const markdown = new Markdown("```ts\nx\n```", 0, 0, countingTheme(calls)); + markdown.render(80); + markdown.setText("```ts\nx\n```\n\nafter"); + markdown.render(40); + assert.deepStrictEqual(calls, ["ts:x"]); + markdown.invalidate(); + markdown.render(40); + assert.deepStrictEqual(calls, ["ts:x", "ts:x"]); + }); + + it("holds rendered blocks weakly, so a full garbage collection releases them", async () => { + setFlagsFromString("--expose-gc"); + const gc = runInNewContext("gc") as () => void; + const calls: string[] = []; + const markdown = new Markdown("```ts\nx\n```", 0, 0, countingTheme(calls)); + markdown.render(80); + // A WeakRef keeps its target alive until the current job ends. + await new Promise((resolve) => setImmediate(resolve)); + gc(); + markdown.render(40); + assert.deepStrictEqual(calls, ["ts:x", "ts:x"]); + }); + + it("renders a block again when the block after it changes its spacing", () => { + const markdown = new Markdown("# Title", 0, 0, defaultMarkdownTheme); + markdown.render(80); + markdown.setText("# Title\nfoo"); + assert.deepStrictEqual( + markdown.render(80).map((line) => stripAnsi(line).trim()), + ["Title", "", "foo"], + ); + }); + + it("follows changed reference link definitions", () => { + setCapabilities({ images: null, trueColor: false, hyperlinks: false }); + const markdown = new Markdown("[a][x]\n\n[x]: https://one.example", 0, 0, defaultMarkdownTheme); + markdown.render(80); + markdown.setText("[a][x]\n\n[x]: https://two.example"); + assert.ok(stripAnsi(markdown.render(80).join("\n")).includes("https://two.example")); + }); + }); + describe("Lists", () => { it("should render simple nested list", () => { const markdown = new Markdown( diff --git a/packages/tui/test/text.test.ts b/packages/tui/test/text.test.ts new file mode 100644 index 000000000..9c2e2bd61 --- /dev/null +++ b/packages/tui/test/text.test.ts @@ -0,0 +1,12 @@ +import assert from "node:assert"; +import { describe, it } from "node:test"; +import { Text } from "../src/components/text.ts"; + +describe("Text", () => { + it("keeps its wrapped lines when the same text is set again", () => { + const text = new Text("hello world", 0, 0); + const lines = text.render(5); + text.setText("hello world"); + assert.strictEqual(text.render(5), lines); + }); +});