From 4678c80cd8f3b70efdb43d5e8a79c0c9f4194377 Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 17 Jun 2026 15:03:32 +0200 Subject: [PATCH] fix(tui): fixed inline image transmit replay for full paint - Adjusted image budget IDs to use a shared 24-bit seed and wrap safely so new budgets no longer collide. - Added transmit-state reset and changed image output ordering so full paints can replay image data after terminal clears. - Updated image and protocol tests to match the new wrapped-id behavior and cursor-save/restore-wrapped transmit sequences. --- .../test/google-gemini-cli-alignment.test.ts | 4 +-- .../test/debug/protocol-probe.test.ts | 9 ++++-- packages/tui/src/components/image.ts | 28 +++++++++++++++++-- packages/tui/src/tui.ts | 20 +++++++------ packages/tui/test/image-budget.test.ts | 23 +++++++++++---- packages/tui/test/image-render.test.ts | 4 +-- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/packages/ai/test/google-gemini-cli-alignment.test.ts b/packages/ai/test/google-gemini-cli-alignment.test.ts index 9b534992e..f8c090a00 100644 --- a/packages/ai/test/google-gemini-cli-alignment.test.ts +++ b/packages/ai/test/google-gemini-cli-alignment.test.ts @@ -313,8 +313,8 @@ describe("Google Gemini CLI alignment", () => { const parts = payload.request.systemInstruction?.parts ?? []; // The antigravity identity header must be injected as the first part. expect(parts[0]?.text).toBe(ANTIGRAVITY_SYSTEM_INSTRUCTION); - // The user-supplied system prompt must appear after the injected parts. - expect(parts.slice(3).some(p => p.text === "my instructions")).toBe(true); + // The user-supplied system prompt must appear after the single injected part. + expect(parts.slice(1).some(p => p.text === "my instructions")).toBe(true); } }); it("adds anthropic-beta for Antigravity Claude reasoning models without relying on id suffix", async () => { diff --git a/packages/coding-agent/test/debug/protocol-probe.test.ts b/packages/coding-agent/test/debug/protocol-probe.test.ts index 2eabac540..a195d42d8 100644 --- a/packages/coding-agent/test/debug/protocol-probe.test.ts +++ b/packages/coding-agent/test/debug/protocol-probe.test.ts @@ -55,8 +55,13 @@ it("uses independent graphics ids for repeated probe panels", () => { const secondBytes = second.render(80).join("\n"); budget.endPass(); - expect(firstBytes).toContain("i=1"); - expect(secondBytes).toContain("i=2"); + const firstMatch = firstBytes.match(/i=(\d+)/); + const secondMatch = secondBytes.match(/i=(\d+)/); + expect(firstMatch).not.toBeNull(); + expect(secondMatch).not.toBeNull(); + const firstId = Number(firstMatch![1]); + const secondId = Number(secondMatch![1]); + expect(secondId).toBe((firstId + 1) & 0xffffff || 1); }); describe("buildLargeTextLines", () => { diff --git a/packages/tui/src/components/image.ts b/packages/tui/src/components/image.ts index 7a88aa8fb..930a93501 100644 --- a/packages/tui/src/components/image.ts +++ b/packages/tui/src/components/image.ts @@ -38,6 +38,11 @@ const RESERVED_IMAGE_ROW = "\x1b[0m"; /** Default count of inline images kept as live graphics before older ones fall back to text. */ export const DEFAULT_MAX_INLINE_IMAGES = 8; +let nextImageBudgetSeed = Math.floor(Math.random() * 0xffffff); +function nextImageIdSeed(): number { + nextImageBudgetSeed = (nextImageBudgetSeed + 0x10000) & 0xffffff; + return nextImageBudgetSeed || 1; +} /** * Bounds how many inline images render as live terminal graphics at once. * @@ -58,7 +63,7 @@ export const DEFAULT_MAX_INLINE_IMAGES = 8; export class ImageBudget { #cap: number; #requestRender: () => void; - #nextId = 1; + #nextId = nextImageIdSeed(); #keyToId = new Map(); /** Display-order image ids observed during the in-flight pass. */ #passIds: number[] = []; @@ -124,11 +129,14 @@ export class ImageBudget { if (key) { const existing = this.#keyToId.get(key); if (existing !== undefined) return existing; - const id = this.#nextId++; + const id = this.#nextId; + this.#nextId = (this.#nextId + 1) & 0xffffff || 1; this.#keyToId.set(key, id); return id; } - return this.#nextId++; + const id = this.#nextId; + this.#nextId = (this.#nextId + 1) & 0xffffff || 1; + return id; } /** @@ -252,6 +260,20 @@ export class ImageBudget { return sequences; } + /** + * Drop transmit tracking so every still-live image re-enqueues its data + * (`a=t`) on the next render. Recovers when the terminal dropped the original + * transmit — e.g. Ghostty discarding graphics sent during its post-startup + * window — where a placement-only replay can never bind a Unicode placeholder. + * Pair with a component invalidate + forced repaint so the data and placement + * re-emit together; keeps no base64 in budget state (the transmit-once design). + */ + forgetTransmitted(): void { + if (this.#transmitted.size === 0 && this.#pendingTransmits.length === 0) return; + this.#transmitted.clear(); + this.#pendingTransmits = []; + } + #reconcile(total: number): void { const desired = this.#cap > 0 ? Math.max(0, total - this.#cap) : 0; if (desired === this.#planned) { diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index d3b56251d..8e5a704fc 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -2655,14 +2655,11 @@ export class TUI extends Container { this.#logRedraw(intent, frameLength, height); // Load newly-displayed image data once, before this frame's placements - // (and any emitter) reference it. `a=t` produces no display, so writing - // it ahead of the synchronized paint is artifact-free. - const imageTransmits = this.#imageBudget.takeTransmits(); - if (imageTransmits.length > 0) { - let transmitBuffer = ""; - for (const seq of imageTransmits) transmitBuffer += seq; - this.terminal.write(transmitBuffer); - } + // reference it. For full paints, the emitter may need to place the + // transmit after a destructive clear (ED2/ED3) but before row replay, so + // build the buffer here and let the emitter decide where it lands. + let imageTransmitBuffer = ""; + for (const seq of this.#imageBudget.takeTransmits()) imageTransmitBuffer += seq; // Purge graphics for images the budget demoted to text. Kitty keeps // images in a store that text clears don't touch; demoted rows still // visible re-render as text and the window diff repaints them. @@ -2677,7 +2674,7 @@ export class TUI extends Container { // 6. Emit. if (intent.kind === "fullPaint") { - this.#emitFullPaint(frame, window, width, height, cursorPos, purgeSequence, { + this.#emitFullPaint(frame, window, width, height, cursorPos, purgeSequence, imageTransmitBuffer, { clearScrollback: intent.clearScrollback, chunkTo, windowTop, @@ -2689,6 +2686,9 @@ export class TUI extends Container { if (!firstPaint && frameLength > height) this.#armPostFullPaintSettle(); return; } + if (imageTransmitBuffer.length > 0) { + this.terminal.write(imageTransmitBuffer); + } this.#emitUpdate(frame, window, width, height, cursorPos, purgeSequence, { chunkTo, windowTop, @@ -3043,6 +3043,7 @@ export class TUI extends Container { height: number, cursorPos: { row: number; col: number } | null, purgeSequence: string, + imageTransmitBuffer: string, options: { clearScrollback: boolean; chunkTo: number; windowTop: number }, ): void { this.#fullRedrawCount += 1; @@ -3059,6 +3060,7 @@ export class TUI extends Container { if (TERMINAL.supportsScreenToScrollback) buffer += "\x1b[22J"; buffer += "\x1b[2J\x1b[H"; } + if (imageTransmitBuffer.length > 0) buffer += imageTransmitBuffer; // DECCARA fills optimize only the rows that stay visible; history-bound // rows are written as full styled strings (their background must // survive in scrollback, which DECCARA cannot reach). diff --git a/packages/tui/test/image-budget.test.ts b/packages/tui/test/image-budget.test.ts index 33ddab97b..7fbde8af1 100644 --- a/packages/tui/test/image-budget.test.ts +++ b/packages/tui/test/image-budget.test.ts @@ -130,6 +130,12 @@ describe("ImageBudget", () => { expect(budget.acquireId()).not.toBe(budget.acquireId()); }); + it("initializes separate budgets with different starting IDs", () => { + const budget1 = new ImageBudget(); + const budget2 = new ImageBudget(); + expect(budget1.acquireId()).not.toBe(budget2.acquireId()); + }); + it("setCap(0) clears a previously applied demotion threshold", () => { const budget = new ImageBudget(2, () => {}); pass(budget, 3); @@ -266,13 +272,13 @@ describe("Image budget integration", () => { const last = lines.at(-1) ?? ""; expect(lines).toHaveLength(4); expect(lines.slice(0, -1)).toEqual(["\x1b[0m", "\x1b[0m", "\x1b[0m"]); - expect(last.startsWith("\x1b[3A")).toBe(true); + expect(last.startsWith("\x1b7\x1b[3A")).toBe(true); + expect(last.endsWith("\x1b8")).toBe(true); expect(last).toContain("\x1b_Ga=p"); expect(last).toContain("C=1"); expect(last).toContain(`i=${id}`); expect(last).toContain("c=4"); expect(last).toContain("r=4"); - expect(last.endsWith("\x1b[3B")).toBe(true); }); it("does not move the cursor around single-row direct Kitty placements", () => { @@ -371,7 +377,7 @@ describe("Image budget + Unicode placeholders", () => { expect(lines.every(l => l.includes(KITTY_PLACEHOLDER))).toBe(true); expect(lines.join("")).not.toContain("\x1b[1A"); // The image id is encoded in the cell foreground color (low 24 bits). - expect(lines[0]).toContain(`38;2;0;0;${id}`); + expect(lines[0]).toContain(`38;2;${(id >> 16) & 0xff};${(id >> 8) & 0xff};${id & 0xff}m`); // Render lines never carry the base64 — data goes via the one-time transmit. expect(lines.join("")).not.toContain(BASE64_ONE_PIXEL_PNG); const transmits = [...budget.takeTransmits()]; @@ -406,6 +412,7 @@ describe("Image budget + Unicode placeholders", () => { describe("TUI inline-image budget", () => { const originalProtocol = TERMINAL.imageProtocol; + const originalTerminalId = terminal.id; let originalCellDims: CellDimensions; let monotonicNow = 0; @@ -413,6 +420,10 @@ describe("TUI inline-image budget", () => { originalCellDims = { ...getCellDimensions() }; setCellDimensions({ widthPx: 10, heightPx: 10 }); terminal.imageProtocol = ImageProtocol.Kitty; + // Pin a non-Ghostty id by default so the Ghostty one-shot image re-submit + // (which re-sends `a=t` data) never fires in the generic budget tests; the + // dedicated Ghostty tests opt in by setting `terminal.id = "ghostty"`. + terminal.id = "xterm"; monotonicNow = 0; // Advance one full 30fps frame (>1000/30ms) per tick so the render // throttle computes a zero delay and every requestRender flushes inline. @@ -426,6 +437,7 @@ describe("TUI inline-image budget", () => { vi.restoreAllMocks(); setCellDimensions(originalCellDims); terminal.imageProtocol = originalProtocol; + terminal.id = originalTerminalId; }); async function settle(term: VirtualTerminal): Promise { @@ -475,10 +487,9 @@ describe("TUI inline-image budget", () => { await settle(term); const output = writes.join(""); - expect(output).toContain("\x1b[3A"); + expect(output).toContain("\x1b7\x1b[3A"); expect(output).toContain("C=1"); - expect(output).toContain("\x1b[3B"); - + expect(output).toContain("\x1b8"); const viewport = term.getViewport().map(line => line.trimEnd()); expect(viewport.slice(0, 5)).toEqual(["", "", "", "", "after-image"]); expect(viewport.slice(0, 4).some(line => line.includes("after-image"))).toBe(false); diff --git a/packages/tui/test/image-render.test.ts b/packages/tui/test/image-render.test.ts index b247b4782..c512fb873 100644 --- a/packages/tui/test/image-render.test.ts +++ b/packages/tui/test/image-render.test.ts @@ -195,12 +195,12 @@ describe("terminal image rendering", () => { expect(lines).toHaveLength(3); expect(lines.slice(0, -1)).toEqual(["\x1b[0m", "\x1b[0m"]); - expect(imageLine.startsWith("\x1b[2A")).toBe(true); + expect(imageLine.startsWith("\x1b7\x1b[2A")).toBe(true); expect(imageLine).toContain("\x1b_Ga=T"); expect(imageLine).toContain("C=1"); expect(imageLine).toContain("c=3"); expect(imageLine).toContain("r=3"); - expect(imageLine.endsWith("\x1b[2B")).toBe(true); + expect(imageLine.endsWith("\x1b8")).toBe(true); }); it("does not emit cursor movement around single-row direct Kitty output", () => {