From 8fbd21ebf6b7da40f14a875d285d42b4f9b1b588 Mon Sep 17 00:00:00 2001 From: DarkPhilosophy <19309990+DarkPhilosophy@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:55:35 +0300 Subject: [PATCH] fix(tui): do not coalesce across incomplete extended-color SGR A semicolon-form extended color with a missing component (e.g. 38;2;255;0 lacking blue, or 38;5 lacking the palette index) has ambiguous arity: if a following SGR is concatenated onto it, the next code is absorbed as the missing channel/index and the rendered color changes. Stop a merge run at any parameter list that ends mid extended-color so the following sequence keeps its original framing. Complete colors and the self-delimiting colon form still merge, so the win is unchanged on real transcripts (34.3% SGR reduction on the sampled capture). Adds regression tests for incomplete truecolor/indexed runs and the complete-color merge. --- packages/tui/CHANGELOG.md | 2 +- packages/tui/src/tui.ts | 54 +++++++++++++++++++++----- packages/tui/test/sgr-coalesce.test.ts | 32 +++++++++++++++ 3 files changed, 78 insertions(+), 10 deletions(-) diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index ff4fdf927..7f8a1ad0b 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changed -- Coalesced byte-adjacent SGR sequences in emitted lines into a single `CSI … m`. The component tree styles each span as `text`, so adjacent spans emit runs of back-to-back SGR sequences (e.g. a `CSI 39 m` fg-reset immediately followed by the next span's `CSI 38;2;r;g;b m`); merging the run is behavior-preserving because SGR parameters apply left-to-right regardless of framing. On a real transcript this drops ~30-40% of all SGR sequences, cutting the per-frame byte volume and SGR-dispatch count a slow terminal engine (e.g. xterm.js/WebGL under a large viewport) must process. Each emitted sequence is capped at 16 parameter tokens so a long adjacent run is split across several valid CSIs instead of overflowing a terminal's parameter buffer (xterm.js caps at 32 and silently truncates, corrupting colors). Disable with `PI_NO_SGR_COALESCE=1`. +- Coalesced byte-adjacent SGR sequences in emitted lines into a single `CSI … m`. The component tree styles each span as `text`, so adjacent spans emit runs of back-to-back SGR sequences (e.g. a `CSI 39 m` fg-reset immediately followed by the next span's `CSI 38;2;r;g;b m`); merging the run is behavior-preserving because SGR parameters apply left-to-right regardless of framing. On a real transcript this drops ~30-40% of all SGR sequences, cutting the per-frame byte volume and SGR-dispatch count a slow terminal engine (e.g. xterm.js/WebGL under a large viewport) must process. Each emitted sequence is capped at 16 parameter tokens so a long adjacent run is split across several valid CSIs instead of overflowing a terminal's parameter buffer (xterm.js caps at 32 and silently truncates, corrupting colors). A run is never extended past a parameter list that ends in an incomplete semicolon-form extended color (`38/48/58;2` missing a channel or `;5` missing the index), so a following code can't be absorbed as the missing component. Disable with `PI_NO_SGR_COALESCE=1`. ## [16.0.3] - 2026-06-16 diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index 9a4274f7c..d3b56251d 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -666,6 +666,38 @@ function isSgrParamByte(c: number): boolean { return (c >= 0x30 && c <= 0x39) || c === CC_SEMI || c === CC_COLON; } +// True when a parameter list ends mid extended-color spec in the ambiguous +// semicolon form: `38/48/58;2` with fewer than three channel values, or +// `38/48/58;5` with no palette index. Concatenating another list after such a +// run would let the next code be absorbed as the missing channel/index (e.g. +// `38;2;255;0` + `31` → `38;2;255;0;31`, where `31` becomes blue instead of a +// standalone fg-red), changing the rendered color. The self-delimiting colon +// form (`38:2::r:g:b`) is unambiguous — its tokens never equal a bare `38`, so +// the scan treats it as a complete unit and merging stays safe. +function endsWithIncompleteExtendedColor(params: string): boolean { + const t = params.split(";"); + let i = 0; + while (i < t.length) { + const tok = t[i]; + if (tok === "38" || tok === "48" || tok === "58") { + const mode = t[i + 1]; + if (mode === undefined) return true; // introducer with no mode + if (mode === "2") { + if (i + 4 >= t.length) return true; // missing r/g/b + i += 5; + continue; + } + if (mode === "5") { + if (i + 2 >= t.length) return true; // missing index + i += 3; + continue; + } + } + i += 1; + } + return false; +} + /** * Merge runs of byte-adjacent SGR sequences (`CSI [0-9;:]* m`) into one. Only * CSI-SGR sequences are touched; text, cursor moves, OSC, hyperlinks and image @@ -703,16 +735,19 @@ export function coalesceAdjacentSgr(line: string): string { } if (params.length > 1) { out += line.slice(copiedUpto, i); - // Emit the merged run, but cap each emitted CSI at MERGE_TOKEN_CAP - // parameter tokens. SGR params apply left-to-right regardless of how - // they are grouped across adjacent CSIs, so splitting a long run into - // several capped sequences stays behavior-preserving — while a single - // unbounded merge would overflow a terminal's CSI parameter buffer - // (xterm.js caps at 32 and silently truncates the rest, corrupting - // the colors). Empty params (`CSI m`) mean a full reset; normalize to - // `0` so the merged list stays unambiguous. + // Emit the merged run, but flush the current group before appending a + // list when (a) the previous list ended mid extended-color, so the + // next code cannot be absorbed as its missing channel/index, or (b) + // the token count would exceed MERGE_TOKEN_CAP. SGR params apply + // left-to-right regardless of how they are grouped across adjacent + // CSIs, so a capped/guarded split stays behavior-preserving — while a + // single unbounded merge would overflow a terminal's CSI parameter + // buffer (xterm.js caps at 32 and silently truncates the rest, + // corrupting colors). Empty params (`CSI m`) mean a full reset; + // normalize to `0` so the merged list stays unambiguous. let group = ""; let groupTokens = 0; + let groupOpenSafe = true; for (let q = 0; q < params.length; q++) { const norm = params[q]!.length === 0 ? "0" : params[q]!; let tk = 1; @@ -720,13 +755,14 @@ export function coalesceAdjacentSgr(line: string): string { const cc = norm.charCodeAt(z); if (cc === CC_SEMI || cc === CC_COLON) tk++; } - if (groupTokens > 0 && groupTokens + tk > MERGE_TOKEN_CAP) { + if (groupTokens > 0 && (!groupOpenSafe || groupTokens + tk > MERGE_TOKEN_CAP)) { out += `\x1b[${group}m`; group = ""; groupTokens = 0; } group += group.length === 0 ? norm : `;${norm}`; groupTokens += tk; + groupOpenSafe = !endsWithIncompleteExtendedColor(norm); } if (group.length > 0) out += `\x1b[${group}m`; copiedUpto = k; diff --git a/packages/tui/test/sgr-coalesce.test.ts b/packages/tui/test/sgr-coalesce.test.ts index fb7c9eb7a..cc859eb37 100644 --- a/packages/tui/test/sgr-coalesce.test.ts +++ b/packages/tui/test/sgr-coalesce.test.ts @@ -74,4 +74,36 @@ describe("coalesceAdjacentSgr", () => { expect(b.getViewportRowForegroundColumns(0)).toEqual(a.getViewportRowForegroundColumns(0)); expect(b.getViewportRowBackgroundColumns(0)).toEqual(a.getViewportRowBackgroundColumns(0)); }); + + it("does not merge across an incomplete truecolor introducer (missing channel)", () => { + // `38;2;255;0` is missing its blue channel. Concatenating the next list + // would let `31` be consumed as that channel (`38;2;255;0;31`) instead of + // staying a standalone fg-red, changing the rendered color. + const input = "\x1b[38;2;255;0m\x1b[31mX"; + expect(coalesceAdjacentSgr(input)).toBe(input); + }); + + it("does not merge across an incomplete indexed-color introducer (missing index)", () => { + // `38;5` (256-color) is missing its palette index; `31` must not be absorbed. + const input = "\x1b[38;5m\x1b[31mX"; + expect(coalesceAdjacentSgr(input)).toBe(input); + }); + + it("still merges a complete extended color followed by another code", () => { + // `38;2;255;0;0` consumes exactly r,g,b; a trailing `31` then starts fresh, + // so concatenation stays behavior-preserving and the merge win is kept. + const input = "\x1b[38;2;255;0;0m\x1b[31mX"; + expect(coalesceAdjacentSgr(input)).toBe("\x1b[38;2;255;0;0;31mX"); + }); + + it("renders malformed extended-color runs identically (no channel absorption)", () => { + const input = "\x1b[38;2;255;0m\x1b[31mX"; + const out = coalesceAdjacentSgr(input); + const a = new VirtualTerminal(80, 4); + const b = new VirtualTerminal(80, 4); + a.write(input); + b.write(out); + expect(b.getViewport()).toEqual(a.getViewport()); + expect(b.getViewportRowForegroundColumns(0)).toEqual(a.getViewportRowForegroundColumns(0)); + }); });