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
This commit is contained in:
roboomp
2026-07-11 00:06:28 +00:00
parent 7aa1d581c6
commit e41b32c87f
4 changed files with 273 additions and 26 deletions
@@ -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, "")),
);
});
}
});
@@ -1,6 +1,10 @@
import { describe, expect, it } from "bun:test"; import { describe, expect, it } from "bun:test";
import { TranscriptContainer } from "@oh-my-pi/pi-coding-agent/modes/components/transcript-container"; 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 { class MutableLiveBlock implements Component {
#lines: string[]; #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)", () => { describe("transcript streaming commit (assistant text)", () => {
it("commits only the declared settled head while the trailing line grows", () => { it("commits only the declared settled head while the trailing line grows", () => {
const chat = new TranscriptContainer(); const chat = new TranscriptContainer();
@@ -41,4 +102,50 @@ describe("transcript streaming commit (assistant text)", () => {
expect(chat.getNativeScrollbackLiveRegionStart()).toBe(2); 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();
}
});
}); });
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased] ## [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 ## [16.4.1] - 2026-07-10
### Added ### Added
+109 -25
View File
@@ -937,6 +937,11 @@ interface StreamPrefixLineCache extends RenderSignature {
tokenCount: number; tokenCount: number;
lines: readonly string[]; lines: readonly string[];
} }
interface StreamingDiffLineCache extends RenderSignature {
lang: string | undefined;
text: string;
lines: readonly string[];
}
export class Markdown implements Component { export class Markdown implements Component {
#text: string; #text: string;
@@ -979,8 +984,12 @@ export class Markdown implements Component {
// True while #renderStreamingContentLines renders the frozen token range: // True while #renderStreamingContentLines renders the frozen token range:
// frozen code blocks highlight even in transient mode so their bytes match // 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 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; #renderingFrozenPrefix = false;
#streamingDiffLineCache?: StreamingDiffLineCache;
#activeRenderSignature?: RenderSignature;
#ignoreTight = false; #ignoreTight = false;
@@ -1190,9 +1199,15 @@ export class Markdown implements Component {
// Parse markdown to HTML-like tokens // Parse markdown to HTML-like tokens
const tokens = this.#lexTokens(normalizedText); const tokens = this.#lexTokens(normalizedText);
const contentLines = this.transientRenderCache let contentLines: string[];
? this.#renderStreamingContentLines(tokens, normalizedText, signature, contentWidth) this.#activeRenderSignature = signature;
: this.#renderContentLines(tokens, 0, tokens.length, contentWidth, 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); const emptyLines = this.#renderEmptyPaddingLines(signature);
// Combine top padding, content, and bottom padding // Combine top padding, content, and bottom padding
@@ -1386,6 +1401,92 @@ export class Markdown implements Component {
return contentLines; 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[] { #renderEmptyPaddingLines(signature: RenderSignature): string[] {
const emptyLine = padding(signature.width); const emptyLine = padding(signature.width);
const emptyLines: string[] = []; const emptyLines: string[] = [];
@@ -1563,17 +1664,8 @@ export class Markdown implements Component {
const codeIndent = padding(this.#codeBlockIndent); const codeIndent = padding(this.#codeBlockIndent);
lines.push(this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`)); lines.push(this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`));
if (this.#theme.highlightCode && (!this.transientRenderCache || this.#renderingFrozenPrefix)) { for (const bodyLine of this.#renderCodeBodyLines(token, codeIndent)) {
const highlightedLines = this.#theme.highlightCode(token.text, token.lang); lines.push(bodyLine);
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)}`);
}
} }
lines.push(this.#theme.codeBlockBorder("```")); lines.push(this.#theme.codeBlockBorder("```"));
if (nextTokenType && nextTokenType !== "space") { if (nextTokenType && nextTokenType !== "space") {
@@ -1953,16 +2045,8 @@ export class Markdown implements Component {
// Code block in list item // Code block in list item
const codeIndent = padding(this.#codeBlockIndent); const codeIndent = padding(this.#codeBlockIndent);
lines.push({ text: this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`), nested: false }); lines.push({ text: this.#theme.codeBlockBorder(`\`\`\`${token.lang || ""}`), nested: false });
if (this.#theme.highlightCode && (!this.transientRenderCache || this.#renderingFrozenPrefix)) { for (const bodyLine of this.#renderCodeBodyLines(token, codeIndent)) {
const highlightedLines = this.#theme.highlightCode(token.text, token.lang); lines.push({ text: bodyLine, nested: false });
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 });
}
} }
lines.push({ text: this.#theme.codeBlockBorder("```"), nested: false }); lines.push({ text: this.#theme.codeBlockBorder("```"), nested: false });
} else if (isMathToken(token)) { } else if (isMathToken(token)) {