From e41b32c87f8b3a665e728eb85a5eec26c7d3eeb0 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 11 Jul 2026 00:06:28 +0000 Subject: [PATCH] fix(tui): highlighted streamed diff scrollback rows Highlighted completed diff and patch fence lines during transient Markdown rendering so rows enter native scrollback with semantic colors before finalization. Added regression coverage for streamed diff scrollback foreground preservation and diff highlighter chunk parity. Fixes #5126 --- .../test/theme-highlight-diff-parity.test.ts | 52 +++++++ .../transcript-streaming-commit-repro.test.ts | 109 +++++++++++++- packages/tui/CHANGELOG.md | 4 + packages/tui/src/components/markdown.ts | 134 ++++++++++++++---- 4 files changed, 273 insertions(+), 26 deletions(-) create mode 100644 packages/coding-agent/test/theme-highlight-diff-parity.test.ts diff --git a/packages/coding-agent/test/theme-highlight-diff-parity.test.ts b/packages/coding-agent/test/theme-highlight-diff-parity.test.ts new file mode 100644 index 000000000..49c83f9b9 --- /dev/null +++ b/packages/coding-agent/test/theme-highlight-diff-parity.test.ts @@ -0,0 +1,52 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { getThemeByName, highlightCode, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; + +const unifiedDiffChunks = [ + [ + "diff --git a/src/example.ts b/src/example.ts", + "index 1234567..89abcde 100644", + "--- a/src/example.ts", + "+++ b/src/example.ts", + "@@ -1,4 +1,5 @@", + ' import { run } from "./run";', + "-const enabled = false;", + "+const enabled = true;", + "+run(enabled);", + ], + [ + "@@ -8,4 +9,4 @@ export function start() {", + " context();", + '-return "old";', + '+return "new";', + "\\ No newline at end of file", + ], +].map(lines => lines.join("\n")); + +const unifiedDiff = unifiedDiffChunks.join("\n"); +const diffLanguages: Array<"diff" | "patch"> = ["diff", "patch"]; + +beforeAll(async () => { + const darkTheme = await getThemeByName("dark"); + if (!darkTheme) throw new Error("Expected dark theme to exist"); + setThemeInstance(darkTheme); +}); + +describe("diff highlighter chunk parity", () => { + for (const lang of diffLanguages) { + it(`highlights newline-complete ${lang} chunks with whole-block visual styles`, () => { + const completeDiff = `${unifiedDiff}\n`; + const wholeBlock = highlightCode(completeDiff, lang); + const chunkedLines = unifiedDiffChunks.flatMap(chunk => { + const highlighted = highlightCode(`${chunk}\n`, lang); + return highlighted.slice(0, -1); + }); + chunkedLines.push(""); + + expect(Bun.stripANSI(wholeBlock.join("\n"))).toBe(completeDiff); + expect(wholeBlock.join("\n")).not.toBe(completeDiff); + expect(chunkedLines.map(line => line.replace(/^\x1b\[39m/u, ""))).toEqual( + wholeBlock.map(line => line.replace(/^\x1b\[39m/u, "")), + ); + }); + } +}); diff --git a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts index 943d5e0c9..404f1598c 100644 --- a/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts +++ b/packages/coding-agent/test/transcript-streaming-commit-repro.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "bun:test"; import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; -import type { Component } from "@oh-my-pi/pi-tui"; +import { type Component, TUI } from "@oh-my-pi/pi-tui"; +import { Markdown, type MarkdownTheme } from "@oh-my-pi/pi-tui/components/markdown"; +import { StressRenderScheduler } from "../../tui/test/render-stress-scheduler"; +import { defaultMarkdownTheme } from "../../tui/test/test-themes.js"; +import { VirtualTerminal } from "../../tui/test/virtual-terminal"; class MutableLiveBlock implements Component { #lines: string[]; @@ -24,6 +28,63 @@ class MutableLiveBlock implements Component { } } +const diffMarkdownTheme: MarkdownTheme = { + ...defaultMarkdownTheme, + codeBlock: text => text, + codeBlockBorder: text => text, + highlightCode: (source, lang) => { + const normalizedLang = lang?.trim().toLowerCase(); + const highlighted: string[] = []; + for (const line of source.split("\n")) { + if (normalizedLang === "diff" && line.startsWith("+")) highlighted.push(`\x1b[32m${line}\x1b[39m`); + else if (normalizedLang === "diff" && line.startsWith("-")) highlighted.push(`\x1b[31m${line}\x1b[39m`); + else highlighted.push(line); + } + return highlighted; + }, +}; + +class StreamingMarkdownBlock implements Component { + #finalized = false; + #markdown = new Markdown("", 0, 0, diffMarkdownTheme); + + setStreamingText(text: string): void { + this.#finalized = false; + this.#markdown.transientRenderCache = true; + this.#markdown.setText(text); + } + + finalize(text: string): void { + this.#finalized = true; + this.#markdown.transientRenderCache = false; + this.#markdown.setText(text); + } + + render(width: number): readonly string[] { + const lines = this.#markdown.render(width); + return lines; + } + + isTranscriptBlockFinalized(): boolean { + const finalized = this.#finalized; + return finalized; + } + + getTranscriptBlockSettledRows(): number { + const rows = this.#markdown.getLastRenderSettledRows(); + return rows; + } +} + +function foregroundColumnsForBufferRow(term: VirtualTerminal, bufferRow: number): number[] { + const before = term.getBufferPosition(); + term.scrollLines(bufferRow - before.viewportY); + const columns = term.getViewportRowForegroundColumns(0); + const after = term.getBufferPosition(); + term.scrollLines(before.viewportY - after.viewportY); + return columns; +} + describe("transcript streaming commit (assistant text)", () => { it("commits only the declared settled head while the trailing line grows", () => { const chat = new TranscriptContainer(); @@ -41,4 +102,50 @@ describe("transcript streaming commit (assistant text)", () => { expect(chat.getNativeScrollbackLiveRegionStart()).toBe(2); }); + + it("keeps diff foreground on rows committed while a streamed fence is still open", async () => { + if (process.platform === "win32") return; + const rows = 6; + const term = new VirtualTerminal(48, rows); + Object.defineProperty(term, "isNativeViewportAtBottom", { configurable: true, value: () => undefined }); + const scheduler = new StressRenderScheduler(); + const tui = new TUI(term, undefined, { renderScheduler: scheduler }); + const chat = new TranscriptContainer(); + const block = new StreamingMarkdownBlock(); + const diffLines = Array.from({ length: 18 }, (_value, index) => { + const sign = index % 2 === 0 ? "+" : "-"; + return `${sign}changed-${String(index).padStart(2, "0")}`; + }); + const openFence = `\`\`\`diff\n${diffLines.join("\n")}\n`; + const closedFence = `${openFence}\`\`\``; + chat.addChild(block); + tui.addChild(chat); + + try { + tui.start(); + await scheduler.drain(term); + + block.setStreamingText(openFence); + tui.requestRender(); + await scheduler.drain(term); + + const streamedRows = term.getScrollBuffer().map(row => Bun.stripANSI(row).trimEnd()); + const streamedRow = streamedRows.findIndex(row => row.includes("+changed-00")); + expect(streamedRow).toBeGreaterThanOrEqual(0); + expect(streamedRow).toBeLessThan(term.getBufferPosition().baseY); + + block.finalize(closedFence); + tui.requestRender(); + await scheduler.drain(term); + + const finalRows = term.getScrollBuffer().map(row => Bun.stripANSI(row).trimEnd()); + const finalRow = finalRows.findIndex(row => row.includes("+changed-00")); + expect(finalRow).toBe(streamedRow); + expect(finalRow).toBeLessThan(term.getBufferPosition().baseY); + expect(foregroundColumnsForBufferRow(term, finalRow).length).toBeGreaterThan(0); + } finally { + tui.stop(); + await term.flush(); + } + }); }); diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 5d1a9980b..d3c3f0716 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed streamed diff code fences retaining unhighlighted rows in native scrollback when long transient blocks leave the viewport before finalization ([#5126](https://github.com/can1357/oh-my-pi/issues/5126)). + ## [16.4.1] - 2026-07-10 ### Added diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index e81cd52aa..dd3f642ed 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -937,6 +937,11 @@ interface StreamPrefixLineCache extends RenderSignature { tokenCount: number; lines: readonly string[]; } +interface StreamingDiffLineCache extends RenderSignature { + lang: string | undefined; + text: string; + lines: readonly string[]; +} export class Markdown implements Component { #text: string; @@ -979,8 +984,12 @@ export class Markdown implements Component { // True while #renderStreamingContentLines renders the frozen token range: // frozen code blocks highlight even in transient mode so their bytes match // the finalized render (they render once into the prefix line cache, so - // the FFI cost is amortized); the volatile tail stays unhighlighted. + // the FFI cost is amortized). The volatile tail normally stays + // unhighlighted; streaming diff fences line-highlight completed rows so + // semantic colors reach native scrollback before rows leave the viewport. #renderingFrozenPrefix = false; + #streamingDiffLineCache?: StreamingDiffLineCache; + #activeRenderSignature?: RenderSignature; #ignoreTight = false; @@ -1190,9 +1199,15 @@ export class Markdown implements Component { // Parse markdown to HTML-like tokens const tokens = this.#lexTokens(normalizedText); - const contentLines = this.transientRenderCache - ? this.#renderStreamingContentLines(tokens, normalizedText, signature, contentWidth) - : this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature); + let contentLines: string[]; + this.#activeRenderSignature = signature; + try { + contentLines = this.transientRenderCache + ? this.#renderStreamingContentLines(tokens, normalizedText, signature, contentWidth) + : this.#renderContentLines(tokens, 0, tokens.length, contentWidth, signature); + } finally { + this.#activeRenderSignature = undefined; + } const emptyLines = this.#renderEmptyPaddingLines(signature); // Combine top padding, content, and bottom padding @@ -1386,6 +1401,92 @@ export class Markdown implements Component { return contentLines; } + #renderCodeBodyLines(token: Token, codeIndent: string): string[] { + const bodyLines: string[] = []; + const tokenText = "text" in token && typeof token.text === "string" ? token.text : ""; + const lang = "lang" in token && typeof token.lang === "string" ? token.lang : undefined; + const normalizedLang = lang?.toLowerCase(); + const canStreamDiff = + this.transientRenderCache && + !this.#renderingFrozenPrefix && + this.#theme.highlightCode && + (normalizedLang === "diff" || normalizedLang === "patch" || normalizedLang === "udiff"); + + if (this.#theme.highlightCode && (!this.transientRenderCache || this.#renderingFrozenPrefix)) { + const highlightedLines = this.#theme.highlightCode(tokenText, lang); + for (const hlLine of highlightedLines) { + bodyLines.push(`${codeIndent}${hlLine}`); + } + return bodyLines; + } + + if (canStreamDiff) { + const lineEnd = tokenText.lastIndexOf("\n"); + if (lineEnd >= 0) { + const completedText = tokenText.slice(0, lineEnd); + if (completedText.length > 0) { + for (const hlLine of this.#highlightStreamingDiffLines(completedText, lang)) { + bodyLines.push(`${codeIndent}${hlLine}`); + } + } + for (const codeLine of tokenText.slice(lineEnd + 1).split("\n")) { + bodyLines.push(`${codeIndent}${this.#theme.codeBlock(codeLine)}`); + } + return bodyLines; + } + } + + for (const codeLine of tokenText.split("\n")) { + bodyLines.push(`${codeIndent}${this.#theme.codeBlock(codeLine)}`); + } + return bodyLines; + } + + #highlightStreamingDiffLines(completedText: string, lang: string | undefined): readonly string[] { + const highlightCode = this.#theme.highlightCode; + if (!highlightCode) return []; + const signature = this.#activeRenderSignature; + const cache = this.#streamingDiffLineCache; + if ( + signature && + cache && + completedText.startsWith(cache.text) && + (cache.text.length === completedText.length || completedText.charCodeAt(cache.text.length) === 0x0a) && + cache.lang === lang && + cache.width === signature.width && + cache.paddingX === signature.paddingX && + cache.paddingY === signature.paddingY && + cache.codeBlockIndent === signature.codeBlockIndent && + cache.themeId === signature.themeId && + cache.defaultTextStyleId === signature.defaultTextStyleId && + cache.imageProtocol === signature.imageProtocol && + cache.hyperlinks === signature.hyperlinks && + cache.textSizing === signature.textSizing && + cache.bgColorProbe === signature.bgColorProbe && + cache.headingProbe === signature.headingProbe + ) { + if (completedText.length === cache.text.length) return cache.lines; + const lines = cache.lines.slice(); + const addedText = completedText.slice(cache.text.length === 0 ? 0 : cache.text.length + 1); + if (addedText.length > 0) { + for (const codeLine of addedText.split("\n")) { + lines.push(...highlightCode(codeLine, lang)); + } + } + this.#streamingDiffLineCache = { ...signature, lang, text: completedText, lines }; + return lines; + } + + const lines: string[] = []; + for (const codeLine of completedText.split("\n")) { + lines.push(...highlightCode(codeLine, lang)); + } + if (signature) { + this.#streamingDiffLineCache = { ...signature, lang, text: completedText, lines }; + } + return lines; + } + #renderEmptyPaddingLines(signature: RenderSignature): string[] { const emptyLine = padding(signature.width); const emptyLines: string[] = []; @@ -1563,17 +1664,8 @@ export class Markdown implements Component { const codeIndent = padding(this.#codeBlockIndent); lines.push(this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`)); - if (this.#theme.highlightCode && (!this.transientRenderCache || this.#renderingFrozenPrefix)) { - const highlightedLines = this.#theme.highlightCode(token.text, token.lang); - for (const hlLine of highlightedLines) { - lines.push(`${codeIndent}${hlLine}`); - } - } else { - // Split code by newlines and style each line - const codeLines = token.text.split("\n"); - for (const codeLine of codeLines) { - lines.push(`${codeIndent}${this.#theme.codeBlock(codeLine)}`); - } + for (const bodyLine of this.#renderCodeBodyLines(token, codeIndent)) { + lines.push(bodyLine); } lines.push(this.#theme.codeBlockBorder("```")); if (nextTokenType && nextTokenType !== "space") { @@ -1953,16 +2045,8 @@ export class Markdown implements Component { // Code block in list item const codeIndent = padding(this.#codeBlockIndent); lines.push({ text: this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`), nested: false }); - if (this.#theme.highlightCode && (!this.transientRenderCache || this.#renderingFrozenPrefix)) { - const highlightedLines = this.#theme.highlightCode(token.text, token.lang); - for (const hlLine of highlightedLines) { - lines.push({ text: `${codeIndent}${hlLine}`, nested: false }); - } - } else { - const codeLines = token.text.split("\n"); - for (const codeLine of codeLines) { - lines.push({ text: `${codeIndent}${this.#theme.codeBlock(codeLine)}`, nested: false }); - } + for (const bodyLine of this.#renderCodeBodyLines(token, codeIndent)) { + lines.push({ text: bodyLine, nested: false }); } lines.push({ text: this.#theme.codeBlockBorder("```"), nested: false }); } else if (isMathToken(token)) {