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)) {