diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c2ade3370..a909ae23b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,10 @@ # Changelog ## [Unreleased] + ### Added +- Added resumption hint printed to stderr on session exit showing command to resume the session (e.g., `Resume this session with claude --resume `) - New `BlobStore` class for content-addressed storage of large binary data (images) externalized from session files - New `getBlobsDir()` function to get path to blob store directory - Support for externalizing large images to blob store during session persistence, reducing JSONL file size @@ -37,6 +39,9 @@ ### Changed +- Modified `--resume` flag to accept optional session ID or path (e.g., `--resume abc123` or `--resume /path/to/session.jsonl`), with session picker shown when no value provided +- Consolidated `--session` flag as an alias for `--resume` with value for improved CLI consistency +- Removed read tool grouping reset logic that was breaking grouping when text or thinking blocks appeared between tool calls - Image persistence now externalizes images ≥1KB to content-addressed blob store instead of compressing inline - Session loading now automatically resolves blob references back to base64 image data - Session forking now resolves blob references in copied entries to ensure data integrity diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index e474aa063..ad31eec0d 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -21,12 +21,11 @@ export interface Args { appendSystemPrompt?: string; thinking?: ThinkingLevel; continue?: boolean; - resume?: boolean; + resume?: string | true; help?: boolean; version?: boolean; mode?: Mode; noSession?: boolean; - session?: string; sessionDir?: string; models?: string[]; tools?: string[]; @@ -76,8 +75,13 @@ export function parseArgs(args: string[], extensionFlags?: Map { // Return the rendered status line for inline preview - const width = this.ctx.ui.getWidth(); + const width = this.ctx.ui.terminal.columns; return this.ctx.statusLine.getTopBorder(width).content; }, onPluginsChanged: () => { diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 6ab39d314..f404ac32e 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -17,6 +17,7 @@ import { } from "@oh-my-pi/pi-tui"; import { $env, isEnoent, logger, postmortem } from "@oh-my-pi/pi-utils"; import chalk from "chalk"; +import { APP_NAME } from "../config"; import { KeybindingsManager } from "../config/keybindings"; import { renderPromptTemplate } from "../config/prompt-templates"; import { type Settings, settings } from "../config/settings"; @@ -391,7 +392,7 @@ export class InteractiveMode implements InteractiveModeContext { } updateEditorTopBorder(): void { - const width = this.ui.getWidth(); + const width = this.ui.terminal.columns; const topBorder = this.statusLine.getTopBorder(width); this.editor.setTopBorder(topBorder); } @@ -726,14 +727,26 @@ export class InteractiveMode implements InteractiveModeContext { await this.session.emitCustomToolSessionEvent("shutdown"); if (this.isInitialized) { - await this.ui.waitForRender(); + this.ui.requestRender(true); } + // Wait for any pending renders to complete + // requestRender() uses process.nextTick(), so we wait one tick + await new Promise(resolve => process.nextTick(resolve)); + // Drain any in-flight Kitty key release events before stopping. // This prevents escape sequences from leaking to the parent shell over slow SSH. await this.ui.terminal.drainInput(1000); this.stop(); + + // Print resumption hint if this is a persisted session + const sessionId = this.sessionManager.getSessionId(); + const sessionFile = this.sessionManager.getSessionFile(); + if (sessionId && sessionFile) { + process.stderr.write(`\n${chalk.dim(`Resume this session with ${APP_NAME} --resume ${sessionId}`)}\n`); + } + await postmortem.quit(0); } diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 8c1447749..382e3f80f 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -204,13 +204,6 @@ export class UiHelpers { // Render tool call components for (const content of message.content) { if (content.type !== "toolCall") { - // Text/thinking blocks between tool calls break read grouping - if ( - (content.type === "text" && content.text?.trim()) || - (content.type === "thinking" && (content as any).thinking?.trim()) - ) { - readGroup = null; - } continue; } diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index ff4649ca3..b45183680 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -1265,7 +1265,9 @@ export class SessionManager { if (!this.persist || !this.sessionFile) return; await this._queuePersistTask(async () => { await this._closePersistWriterInternal(); - const entries = await Promise.all(this.fileEntries.map(entry => prepareEntryForPersistence(entry, this.blobStore))); + const entries = await Promise.all( + this.fileEntries.map(entry => prepareEntryForPersistence(entry, this.blobStore)), + ); await this._writeEntriesAtomically(entries); this.flushed = true; }); diff --git a/packages/coding-agent/src/tools/render-utils.ts b/packages/coding-agent/src/tools/render-utils.ts index bf5a3303a..de0a45984 100644 --- a/packages/coding-agent/src/tools/render-utils.ts +++ b/packages/coding-agent/src/tools/render-utils.ts @@ -8,7 +8,7 @@ import * as os from "node:os"; import { type Ellipsis, truncateToWidth } from "@oh-my-pi/pi-tui"; import type { Theme } from "../modes/theme/theme"; -export { Ellipsis, truncateToWidth } from "@oh-my-pi/pi-tui"; +export { Ellipsis, replaceTabs, truncateToWidth } from "@oh-my-pi/pi-tui"; // ============================================================================= // Standardized Display Constants @@ -672,10 +672,6 @@ export function wrapBrackets(text: string, theme: Theme): string { return `${theme.format.bracketLeft}${text}${theme.format.bracketRight}`; } -export function replaceTabs(text: string): string { - return text.replace(/\t/g, " "); -} - function pluralize(label: string, count: number): string { if (count === 1) return label; if (/(?:ch|sh|s|x|z)$/i.test(label)) return `${label}es`; diff --git a/packages/coding-agent/src/tui/tree-list.ts b/packages/coding-agent/src/tui/tree-list.ts index 79f25110c..6e0a695d3 100644 --- a/packages/coding-agent/src/tui/tree-list.ts +++ b/packages/coding-agent/src/tui/tree-list.ts @@ -2,7 +2,7 @@ * Hierarchical tree list rendering helper. */ import type { Theme } from "../modes/theme/theme"; -import { formatMoreItems } from "../tools/render-utils"; +import { formatMoreItems, replaceTabs } from "../tools/render-utils"; import type { TreeContext } from "./types"; import { getTreeBranch, getTreeContinuePrefix } from "./utils"; @@ -35,14 +35,14 @@ export function renderTreeList(options: TreeListOptions, theme: Theme): st const rendered = renderItem(items[i], context); if (Array.isArray(rendered)) { if (rendered.length === 0) continue; - lines.push(`${prefix}${rendered[0]}`); + lines.push(`${prefix}${replaceTabs(rendered[0])}`); for (let j = 1; j < rendered.length; j++) { - lines.push(`${continuePrefix}${rendered[j]}`); + lines.push(`${continuePrefix}${replaceTabs(rendered[j])}`); } } else { - lines.push(`${prefix}${rendered}`); + lines.push(`${prefix}${replaceTabs(rendered)}`); + } } - } if (!expanded && items.length > maxItems) { const remaining = items.length - maxItems; diff --git a/packages/coding-agent/test/args.test.ts b/packages/coding-agent/test/args.test.ts index bcadb645e..6e738d48f 100644 --- a/packages/coding-agent/test/args.test.ts +++ b/packages/coding-agent/test/args.test.ts @@ -67,6 +67,22 @@ describe("parseArgs", () => { const result = parseArgs(["-r"]); expect(result.resume).toBe(true); }); + + test("parses --resume with session ID", () => { + const result = parseArgs(["--resume", "abc123"]); + expect(result.resume).toBe("abc123"); + }); + + test("parses -r with session path", () => { + const result = parseArgs(["-r", "/path/to/session.jsonl"]); + expect(result.resume).toBe("/path/to/session.jsonl"); + }); + + test("--resume without value before another flag stays boolean", () => { + const result = parseArgs(["--resume", "--model", "opus"]); + expect(result.resume).toBe(true); + expect(result.model).toBe("opus"); + }); }); describe("flags with values", () => { @@ -105,9 +121,9 @@ describe("parseArgs", () => { expect(result.mode).toBe("rpc"); }); - test("parses --session", () => { + test("parses --session as alias for --resume", () => { const result = parseArgs(["--session", "/path/to/session.jsonl"]); - expect(result.session).toBe("/path/to/session.jsonl"); + expect(result.resume).toBe("/path/to/session.jsonl"); }); test("parses --export", () => { diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index baf9436a4..679698313 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -1,6 +1,11 @@ # Changelog ## [Unreleased] + +### Breaking Changes + +- Removed `getCursorPosition()` method from Component interface and implementations, eliminating hardware cursor positioning support + ### Added - Added sticky column behavior for vertical cursor movement, preserving target column when navigating through lines of varying lengths @@ -10,6 +15,10 @@ ### Changed +- Changed default value of `PI_HARDWARE_CURSOR` environment variable from implicit true to explicit `"1"` for clarity +- Changed default value of `PI_CLEAR_ON_SHRINK` environment variable from implicit false to explicit `"0"` for clarity +- Changed TUI to clear screen on startup to prevent shell prompts and status messages from bleeding into the first rendered frame +- Refactored full-render logic into reusable helper function to reduce code duplication across multiple render paths - Changed autocomplete to include hidden paths but filter out `.git` and its contents - Changed Input component to properly handle surrogate pairs in Unicode text, preventing cursor display corruption with emoji and multi-byte characters - Changed Editor to use `setCursorCol()` for all cursor column updates, enabling sticky column tracking @@ -22,6 +31,7 @@ ### Fixed +- Fixed rendering of extra blank lines when content shrinks by improving cursor positioning logic during line deletion - Fixed cursor display position in Input component when scrolling horizontally through long text - Fixed Kitty keyboard protocol disable sequence to use safe write method, preventing potential output buffering issues - Fixed unnecessary full-screen redraws when changes occur in out-of-view components (e.g., spinners), reducing terminal scroll events and improving performance on slower connections diff --git a/packages/tui/bench/width.ts b/packages/tui/bench/width.ts index cf46510d3..707e4cf45 100644 --- a/packages/tui/bench/width.ts +++ b/packages/tui/bench/width.ts @@ -4,7 +4,7 @@ * Run: bun packages/tui/bench/visible-width.ts */ import { visibleWidth as nativeVisibleWidth } from "@oh-my-pi/pi-natives"; -import { visibleWidthRaw as hybridVisibleWidth } from "../src/utils"; +import { visibleWidthRaw as hybridVisibleWidth, replaceTabs } from "../src/utils"; const ITERATIONS = 10_000; const WARMUP = 500; @@ -52,12 +52,7 @@ const samples = { // Bun.stringWidth with ANSI stripping (what hybrid uses for short strings) function bunStringWidth(str: string): number { if (str.length === 0) return 0; - - let clean = str; - if (str.includes("\t")) { - clean = clean.replace(/\t/g, " "); - } - return Bun.stringWidth(clean); + return Bun.stringWidth(replaceTabs(str)); } interface BenchResult { diff --git a/packages/tui/src/components/box.ts b/packages/tui/src/components/box.ts index 40af24b1f..51f42408d 100644 --- a/packages/tui/src/components/box.ts +++ b/packages/tui/src/components/box.ts @@ -1,5 +1,5 @@ import type { Component } from "../tui"; -import { applyBackgroundToLine, padding, truncateToWidth, visibleWidth } from "../utils"; +import { applyBackgroundToLine, padding, visibleWidth } from "../utils"; type Cache = { key: bigint; @@ -79,7 +79,7 @@ export class Box implements Component { for (const child of this.children) { const lines = child.render(contentWidth); for (const line of lines) { - childLines.push(leftPad + (visibleWidth(line) > contentWidth ? truncateToWidth(line, contentWidth) : line)); + childLines.push(leftPad + line); } } diff --git a/packages/tui/src/components/editor.ts b/packages/tui/src/components/editor.ts index 523ecd75c..6c09bbf9b 100644 --- a/packages/tui/src/components/editor.ts +++ b/packages/tui/src/components/editor.ts @@ -619,42 +619,6 @@ export class Editor implements Component, Focusable { return result; } - getCursorPosition(width: number): { row: number; col: number } | null { - if (!this.useTerminalCursor) return null; - - const paddingX = this.getEditorPaddingX(); - const borderWidth = paddingX + 1; - const layoutWidth = this.getLayoutWidth(width, paddingX); - if (layoutWidth <= 0) return null; - - const layoutLines = this.layoutText(layoutWidth); - const visibleContentHeight = this.getVisibleContentHeight(layoutLines.length); - this.updateScrollOffset(layoutWidth, layoutLines, visibleContentHeight); - - for (let i = 0; i < layoutLines.length; i++) { - if (i < this.scrollOffset || i >= this.scrollOffset + visibleContentHeight) continue; - const layoutLine = layoutLines[i]; - if (!layoutLine || !layoutLine.hasCursor || layoutLine.cursorPos === undefined) continue; - - const lineWidth = visibleWidth(layoutLine.text); - const isCursorAtLineEnd = layoutLine.cursorPos === layoutLine.text.length; - - if (isCursorAtLineEnd && lineWidth >= layoutWidth && layoutLine.text.length > 0) { - const graphemes = [...segmenter.segment(layoutLine.text)]; - const lastGrapheme = graphemes[graphemes.length - 1]?.segment || ""; - const lastWidth = visibleWidth(lastGrapheme) || 1; - const colOffset = borderWidth + Math.max(0, lineWidth - lastWidth); - return { row: 1 + i - this.scrollOffset, col: colOffset }; - } - - const before = layoutLine.text.slice(0, layoutLine.cursorPos); - const colOffset = borderWidth + visibleWidth(before); - return { row: 1 + i - this.scrollOffset, col: colOffset }; - } - - return null; - } - handleInput(data: string): void { const kb = getEditorKeybindings(); diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index 4b4ccba46..a70ee3d7b 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -3,7 +3,7 @@ import type { MermaidImage } from "../mermaid"; import type { SymbolTheme } from "../symbols"; import { encodeITerm2, encodeKitty, getCellDimensions, ImageProtocol, TERMINAL } from "../terminal-capabilities"; import type { Component } from "../tui"; -import { applyBackgroundToLine, padding, visibleWidth, wrapTextWithAnsi } from "../utils"; +import { applyBackgroundToLine, padding, replaceTabs, visibleWidth, wrapTextWithAnsi } from "../utils"; /** * Default text styling for markdown content. @@ -120,11 +120,7 @@ export class Markdown implements Component { } // Replace tabs with 3 spaces for consistent rendering - let normalizedText = this.text.replace(/\t/g, " "); - - // Fix inline code fences: text:```lang or text```lang should have newline before ``` - // This handles malformed markdown from LLM thinking output - normalizedText = normalizedText.replace(/([^\n])```(\w*)\n/g, "$1\n```$2\n"); + const normalizedText = replaceTabs(this.text); // Parse markdown to HTML-like tokens const tokens = marked.lexer(normalizedText); @@ -168,8 +164,10 @@ export class Markdown implements Component { if (bgFn) { contentLines.push(applyBackgroundToLine(lineWithMargins, width, bgFn)); } else { - // No background - don't pad (avoids trailing spaces when copying) - contentLines.push(lineWithMargins); + // No background - just pad to width + const visibleLen = visibleWidth(lineWithMargins); + const paddingNeeded = Math.max(0, width - visibleLen); + contentLines.push(lineWithMargins + padding(paddingNeeded)); } } diff --git a/packages/tui/src/components/text.ts b/packages/tui/src/components/text.ts index 269bd54d8..020334e2c 100644 --- a/packages/tui/src/components/text.ts +++ b/packages/tui/src/components/text.ts @@ -1,5 +1,5 @@ import type { Component } from "../tui"; -import { applyBackgroundToLine, padding, wrapTextWithAnsi } from "../utils"; +import { applyBackgroundToLine, padding, replaceTabs, visibleWidth, wrapTextWithAnsi } from "../utils"; /** * Text component - displays multi-line text with word wrapping @@ -62,7 +62,7 @@ export class Text implements Component { } // Replace tabs with 3 spaces - const normalizedText = this.text.replace(/\t/g, " "); + const normalizedText = replaceTabs(this.text); // Calculate content width (subtract left/right margins) const contentWidth = Math.max(1, width - this.paddingX * 2); @@ -83,8 +83,10 @@ export class Text implements Component { if (this.customBgFn) { contentLines.push(applyBackgroundToLine(lineWithMargins, width, this.customBgFn)); } else { - // No background - don't pad (avoids trailing spaces when copying) - contentLines.push(lineWithMargins); + // No background - just pad to width with spaces + const visibleLen = visibleWidth(lineWithMargins); + const paddingNeeded = Math.max(0, width - visibleLen); + contentLines.push(lineWithMargins + padding(paddingNeeded)); } } diff --git a/packages/tui/src/index.ts b/packages/tui/src/index.ts index 6a9b3c44a..4bb911dc6 100644 --- a/packages/tui/src/index.ts +++ b/packages/tui/src/index.ts @@ -62,4 +62,4 @@ export { emergencyTerminalRestore, ProcessTerminal, type Terminal } from "./term export * from "./terminal-capabilities"; export { type Component, Container, type OverlayHandle, type SizeValue, TUI } from "./tui"; // Utilities -export { Ellipsis, padding, truncateToWidth, visibleWidth, wrapTextWithAnsi } from "./utils"; +export { Ellipsis, padding, replaceTabs, truncateToWidth, visibleWidth, wrapTextWithAnsi } from "./utils"; diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index cebe2a562..8469ba01f 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -4,11 +4,10 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { $env } from "@oh-my-pi/pi-utils"; import { isKeyRelease, matchesKey } from "./keys"; import type { Terminal } from "./terminal"; import { setCellDimensions, TERMINAL } from "./terminal-capabilities"; -import { extractSegments, padding, sliceByColumn, sliceWithWidth, visibleWidth } from "./utils"; +import { extractSegments, sliceByColumn, sliceWithWidth, visibleWidth } from "./utils"; /** * Component interface - all components must implement this @@ -32,11 +31,6 @@ export interface Component { */ wantsKeyRelease?: boolean; - /** - * Optional cursor position within the rendered output (0-based row/col). - */ - getCursorPosition?(width: number): { row: number; col: number } | null; - /** * Invalidate any cached rendering state. * Called when theme changes or when component needs to re-render from scratch. @@ -188,19 +182,6 @@ export class Container implements Component { } } - getCursorPosition(width: number): { row: number; col: number } | null { - let rowOffset = 0; - for (const child of this.children) { - const lines = child.render(width); - const childCursor = child.getCursorPosition?.(width) ?? null; - if (childCursor) { - return { row: rowOffset + childCursor.row, col: childCursor.col }; - } - rowOffset += lines.length; - } - return null; - } - render(width: number): string[] { const lines: string[] = []; for (const child of this.children) { @@ -222,13 +203,12 @@ export class TUI extends Container { /** Global callback for debug key (Shift+Ctrl+D). Called before input is forwarded to focused component. */ public onDebug?: () => void; private renderRequested = false; - private rendering = false; private cursorRow = 0; // Logical cursor row (end of rendered content) private hardwareCursorRow = 0; // Actual terminal cursor row (may differ due to IME positioning) private inputBuffer = ""; // Buffer for parsing terminal responses private cellSizeQueryPending = false; - private showHardwareCursor = $env.PI_HARDWARE_CURSOR === "1"; - private clearOnShrink = process.env.PI_CLEAR_ON_SHRINK === "1"; + private showHardwareCursor = process.env.PI_HARDWARE_CURSOR === "1"; + private clearOnShrink = process.env.PI_CLEAR_ON_SHRINK === "1"; // Clear empty rows when content shrinks (default: off) private maxLinesRendered = 0; // Track terminal's working area (max lines ever rendered) private previousViewportTop = 0; // Track previous viewport top for resize-aware cursor moves private fullRedrawCount = 0; @@ -241,7 +221,6 @@ export class TUI extends Container { preFocus: Component | null; hidden: boolean; }[] = []; - private inputQueue: string[] = []; // Queue input during cell size query to avoid interleaving constructor(terminal: Terminal, showHardwareCursor?: boolean) { super(); @@ -426,10 +405,6 @@ export class TUI extends Container { this.terminal.stop(); } - getWidth(): number { - return this.terminal.columns; - } - requestRender(force = false): void { if (force) { this.previousLines = []; @@ -439,54 +414,23 @@ export class TUI extends Container { this.maxLinesRendered = 0; this.previousViewportTop = 0; } - if (this.stopped) return; if (this.renderRequested) return; this.renderRequested = true; process.nextTick(() => { this.renderRequested = false; - if (this.stopped) return; this.doRender(); }); } - async waitForRender(): Promise { - if (!this.renderRequested && !this.rendering) return; - await new Promise(resolve => { - const check = () => { - if (!this.renderRequested && !this.rendering) { - resolve(); - return; - } - setTimeout(check, 0); - }; - check(); - }); - } - private handleInput(data: string): void { // If we're waiting for cell size response, buffer input and parse if (this.cellSizeQueryPending) { this.inputBuffer += data; const filtered = this.parseCellSizeResponse(); if (filtered.length === 0) return; - if (filtered.length > 0) { - this.inputQueue.push(filtered); - } - // Process queued input after cell size response completes - if (!this.cellSizeQueryPending && this.inputQueue.length > 0) { - const queued = this.inputQueue; - this.inputQueue = []; - for (const item of queued) { - this.processInput(item); - } - } - return; + data = filtered; } - this.processInput(data); - } - - private processInput(data: string): void { // Global debug key handler (Shift+Ctrl+D) if (matchesKey(data, "shift+ctrl+d") && this.onDebug) { this.onDebug(); @@ -510,6 +454,7 @@ export class TUI extends Container { // Pass input to focused component (including Ctrl+C) // The focused component can decide how to handle Ctrl+C if (this.focusedComponent?.handleInput) { + // Filter out key release events unless component opts in if (isKeyRelease(data) && !this.focusedComponent.wantsKeyRelease) { return; } @@ -528,17 +473,16 @@ export class TUI extends Container { const heightPx = parseInt(match[1], 10); const widthPx = parseInt(match[2], 10); - // Remove the response from buffer first - this.inputBuffer = this.inputBuffer.replace(responsePattern, ""); - this.cellSizeQueryPending = false; - if (heightPx > 0 && widthPx > 0) { setCellDimensions({ widthPx, heightPx }); // Invalidate all components so images re-render with correct dimensions - // This is safe now because cellSizeQueryPending=false prevents race with render this.invalidate(); this.requestRender(); } + + // Remove the response from buffer + this.inputBuffer = this.inputBuffer.replace(responsePattern, ""); + this.cellSizeQueryPending = false; } // Check if we have a partial cell size response starting (wait for more data) @@ -789,70 +733,6 @@ export class TUI extends Container { return lines; } - /** - * Find and extract cursor position from rendered lines. - * Searches for CURSOR_MARKER, calculates its position, and strips it from the output. - * @param lines - Rendered lines to search - * @param height - Terminal height (to calculate viewport) - * @returns Cursor position { row, col } or null if no marker found - */ - private extractCursorPosition(lines: string[], height: number): { row: number; col: number } | null { - const viewportTop = Math.max(0, lines.length - height); - for (let row = lines.length - 1; row >= viewportTop; row--) { - const line = lines[row]; - const markerIndex = line.indexOf(CURSOR_MARKER); - if (markerIndex !== -1) { - // Calculate visual column (width of text before marker) - const beforeMarker = line.slice(0, markerIndex); - const col = visibleWidth(beforeMarker); - - // Strip marker from the line - lines[row] = line.slice(0, markerIndex) + line.slice(markerIndex + CURSOR_MARKER.length); - - return { row, col }; - } - } - return null; - } - - /** - * Position the hardware cursor for IME candidate window. - * @param cursorPos The cursor position extracted from rendered output, or null - * @param totalLines Total number of rendered lines - */ - private positionHardwareCursor(cursorPos: { row: number; col: number } | null, totalLines: number): void { - if (!cursorPos || totalLines <= 0) { - this.terminal.hideCursor(); - return; - } - - // Clamp cursor position to valid range - const targetRow = Math.max(0, Math.min(cursorPos.row, totalLines - 1)); - const targetCol = Math.max(0, cursorPos.col); - - // Move cursor from current position to target - const rowDelta = targetRow - this.hardwareCursorRow; - let buffer = ""; - if (rowDelta > 0) { - buffer += `\x1b[${rowDelta}B`; // Move down - } else if (rowDelta < 0) { - buffer += `\x1b[${-rowDelta}A`; // Move up - } - // Move to absolute column (1-indexed) - buffer += `\x1b[${targetCol + 1}G`; - - if (buffer) { - this.terminal.write(buffer); - } - - this.hardwareCursorRow = targetRow; - if (this.showHardwareCursor) { - this.terminal.showCursor(); - } else { - this.terminal.hideCursor(); - } - } - /** Splice overlay content into a base line at a specific column. Single-pass optimized. */ private compositeLineAt( baseLine: string, @@ -881,7 +761,14 @@ export class TUI extends Container { // Compose result const r = TUI.SEGMENT_RESET; const result = - base.before + padding(beforePad) + r + overlay.text + padding(overlayPad) + r + base.after + padding(afterPad); + base.before + + " ".repeat(beforePad) + + r + + overlay.text + + " ".repeat(overlayPad) + + r + + base.after + + " ".repeat(afterPad); // CRITICAL: Always verify and truncate to terminal width. // This is the final safeguard against width overflow which would crash the TUI. @@ -897,26 +784,38 @@ export class TUI extends Container { return sliceByColumn(result, 0, totalWidth, true); } - private doRender(): void { - if (this.stopped) return; - // Guard against re-entrant renders (can happen on Windows when Bun.spawnSync - // yields to the event loop during a sync subprocess call) - if (this.rendering) return; - this.rendering = true; - try { - this.doRenderImpl(); - } finally { - this.rendering = false; + /** + * Find and extract cursor position from rendered lines. + * Searches for CURSOR_MARKER, calculates its position, and strips it from the output. + * Only scans the bottom terminal height lines (visible viewport). + * @param lines - Rendered lines to search + * @param height - Terminal height (visible viewport size) + * @returns Cursor position { row, col } or null if no marker found + */ + private extractCursorPosition(lines: string[], height: number): { row: number; col: number } | null { + // Only scan the bottom `height` lines (visible viewport) + const viewportTop = Math.max(0, lines.length - height); + for (let row = lines.length - 1; row >= viewportTop; row--) { + const line = lines[row]; + const markerIndex = line.indexOf(CURSOR_MARKER); + if (markerIndex !== -1) { + // Calculate visual column (width of text before marker) + const beforeMarker = line.slice(0, markerIndex); + const col = visibleWidth(beforeMarker); + + // Strip marker from the line + lines[row] = line.slice(0, markerIndex) + line.slice(markerIndex + CURSOR_MARKER.length); + + return { row, col }; + } } + return null; } - private doRenderImpl(): void { - // Capture terminal dimensions at start to ensure consistency throughout render + private doRender(): void { + if (this.stopped) return; const width = this.terminal.columns; const height = this.terminal.rows; - // NOTE: previousLines.length is the last frame's content length. - // We intentionally key clear-on-shrink logic off maxLinesRendered (working area), - // not previousLines.length. let viewportTop = Math.max(0, this.maxLinesRendered - height); let prevViewportTop = this.previousViewportTop; let hardwareCursorRow = this.hardwareCursorRow; @@ -942,89 +841,62 @@ export class TUI extends Container { // Width changed - need full re-render (line wrapping changes) const widthChanged = this.previousWidth !== 0 && this.previousWidth !== width; - const debugRedraw = process.env.PI_DEBUG_REDRAW === "1"; - const logRedraw = (reason: string): void => { - if (!debugRedraw) return; - const logPath = path.join(os.homedir(), ".pi", "agent", "pi-debug.log"); - const msg = `[${new Date().toISOString()}] fullRender: ${reason} (prev=${this.previousLines.length}, new=${newLines.length}, height=${height})\n`; - try { - fs.mkdirSync(path.dirname(logPath), { recursive: true }); - fs.appendFileSync(logPath, msg); - } catch { - // Best-effort debug logging; must never break rendering. - } - }; - - // First render - just output everything without clearing (assumes clean screen) - if (this.previousLines.length === 0 && !widthChanged) { - logRedraw("first render"); + // Helper to clear scrollback and viewport and render all new lines + const fullRender = (clear: boolean): void => { this.fullRedrawCount += 1; let buffer = "\x1b[?2026h"; // Begin synchronized output + if (clear) buffer += "\x1b[3J\x1b[2J\x1b[H"; // Clear scrollback, screen, and home for (let i = 0; i < newLines.length; i++) { if (i > 0) buffer += "\r\n"; buffer += newLines[i]; } buffer += "\x1b[?2026l"; // End synchronized output this.terminal.write(buffer); - // After rendering N lines, cursor is at end of last line (clamp to 0 for empty) this.cursorRow = Math.max(0, newLines.length - 1); this.hardwareCursorRow = this.cursorRow; - this.maxLinesRendered = Math.max(this.maxLinesRendered, newLines.length); + // Reset max lines when clearing, otherwise track growth + if (clear) { + this.maxLinesRendered = newLines.length; + } else { + this.maxLinesRendered = Math.max(this.maxLinesRendered, newLines.length); + } this.previousViewportTop = Math.max(0, this.maxLinesRendered - height); this.positionHardwareCursor(cursorPos, newLines.length); this.previousLines = newLines; this.previousWidth = width; + }; + + const debugRedraw = process.env.PI_DEBUG_REDRAW === "1"; + const logRedraw = (reason: string): void => { + if (!debugRedraw) return; + const logPath = path.join(os.homedir(), ".pi", "agent", "pi-debug.log"); + const msg = `[${new Date().toISOString()}] fullRender: ${reason} (prev=${this.previousLines.length}, new=${newLines.length}, height=${height})\n`; + fs.appendFileSync(logPath, msg); + }; + + // First render - just output everything without clearing (assumes clean screen) + if (this.previousLines.length === 0 && !widthChanged) { + logRedraw("first render"); + fullRender(false); return; } // Width changed - full re-render (line wrapping changes) if (widthChanged) { logRedraw(`width changed (${this.previousWidth} -> ${width})`); - this.fullRedrawCount += 1; - let buffer = "\x1b[?2026h"; // Begin synchronized output - buffer += "\x1b[2J\x1b[H"; // Clear screen and move cursor to home - for (let i = 0; i < newLines.length; i++) { - if (i > 0) buffer += "\r\n"; - buffer += newLines[i]; - } - buffer += "\x1b[?2026l"; // End synchronized output - this.terminal.write(buffer); - this.cursorRow = Math.max(0, newLines.length - 1); - this.hardwareCursorRow = this.cursorRow; - // Screen was cleared; reset working area to current render. - this.maxLinesRendered = newLines.length; - this.previousViewportTop = Math.max(0, this.maxLinesRendered - height); - this.positionHardwareCursor(cursorPos, newLines.length); - this.previousLines = newLines; - this.previousWidth = width; + fullRender(true); return; } - // Content shrunk below the working area and no overlays - re-render to clear empty rows. - // We compare against maxLinesRendered (working area), not previousLines.length, since - // previousLines will already reflect the shrunk content after one differential render. - // Configurable via setClearOnShrink() or PI_CLEAR_ON_SHRINK env var. + // Content shrunk below the working area and no overlays - re-render to clear empty rows + // (overlays need the padding, so only do this when no overlays are active) + // Configurable via setClearOnShrink() or PI_CLEAR_ON_SHRINK=0 env var if (this.clearOnShrink && newLines.length < this.maxLinesRendered && this.overlayStack.length === 0) { logRedraw(`clearOnShrink (maxLinesRendered=${this.maxLinesRendered})`); - this.fullRedrawCount += 1; - let buffer = "\x1b[?2026h"; // Begin synchronized output - buffer += "\x1b[2J\x1b[H"; // Clear screen and move cursor to home - for (let i = 0; i < newLines.length; i++) { - if (i > 0) buffer += "\r\n"; - buffer += newLines[i]; - } - buffer += "\x1b[?2026l"; // End synchronized output - this.terminal.write(buffer); - this.cursorRow = Math.max(0, newLines.length - 1); - this.hardwareCursorRow = this.cursorRow; - // Screen was cleared; reset working area so we don't clear again next render. - this.maxLinesRendered = newLines.length; - this.previousViewportTop = Math.max(0, this.maxLinesRendered - height); - this.positionHardwareCursor(cursorPos, newLines.length); - this.previousLines = newLines; - this.previousWidth = width; + fullRender(true); return; } + // Find first and last changed lines let firstChanged = -1; let lastChanged = -1; @@ -1070,29 +942,19 @@ export class TUI extends Container { const extraLines = this.previousLines.length - newLines.length; if (extraLines > height) { logRedraw(`extraLines > height (${extraLines} > ${height})`); - this.fullRedrawCount += 1; - let buffer2 = "\x1b[?2026h"; // Begin synchronized output - buffer2 += "\x1b[2J\x1b[H"; // Clear screen and move cursor to home - for (let i = 0; i < newLines.length; i++) { - if (i > 0) buffer2 += "\r\n"; - buffer2 += newLines[i]; - } - buffer2 += "\x1b[?2026l"; // End synchronized output - this.terminal.write(buffer2); - this.cursorRow = Math.max(0, newLines.length - 1); - this.hardwareCursorRow = this.cursorRow; - // Screen was cleared; reset working area to current render. - this.maxLinesRendered = newLines.length; - this.previousViewportTop = Math.max(0, this.maxLinesRendered - height); - this.positionHardwareCursor(cursorPos, newLines.length); - this.previousLines = newLines; - this.previousWidth = width; + fullRender(true); return; } - for (let i = 0; i < extraLines; i++) { - buffer += "\r\n\x1b[2K"; + if (extraLines > 0) { + buffer += "\x1b[1B"; + } + for (let i = 0; i < extraLines; i++) { + buffer += "\r\x1b[2K"; + if (i < extraLines - 1) buffer += "\x1b[1B"; + } + if (extraLines > 0) { + buffer += `\x1b[${extraLines}A`; } - buffer += `\x1b[${extraLines}A`; buffer += "\x1b[?2026l"; this.terminal.write(buffer); this.cursorRow = targetRow; @@ -1109,21 +971,10 @@ export class TUI extends Container { // Use previousLines.length (not maxLinesRendered) to avoid false positives after content shrinks const previousContentViewportTop = Math.max(0, this.previousLines.length - height); if (firstChanged < previousContentViewportTop) { - // Avoid full redraws when changes happen above the viewport (e.g., spinners ticking in - // out-of-view components). Full redraws rewrite the whole screen from the top and - // cause terminal scroll events when content exceeds the terminal height. - if (lastChanged < previousContentViewportTop) { - // All changes are above the viewport — nothing visible to update. - this.maxLinesRendered = Math.max(this.maxLinesRendered, newLines.length); - this.previousViewportTop = Math.max(0, this.maxLinesRendered - height); - this.positionHardwareCursor(cursorPos, newLines.length); - this.previousLines = newLines; - this.previousWidth = width; - return; - } - - // Clip rendering to the visible viewport and fall through to the differential path. - firstChanged = previousContentViewportTop; + // First change is above previous viewport - need full re-render + logRedraw(`firstChanged < viewportTop (${firstChanged} < ${previousContentViewportTop})`); + fullRender(true); + return; } // Render from first changed line to end @@ -1144,7 +995,7 @@ export class TUI extends Container { hardwareCursorRow = moveTargetRow; } - // Move cursor to first changed line + // Move cursor to first changed line (use hardwareCursorRow for actual position) const lineDiff = computeLineDiff(moveTargetRow); if (lineDiff > 0) { buffer += `\x1b[${lineDiff}B`; // Move down @@ -1160,44 +1011,35 @@ export class TUI extends Container { for (let i = firstChanged; i <= renderEnd; i++) { if (i > firstChanged) buffer += "\r\n"; buffer += "\x1b[2K"; // Clear current line - let line = newLines[i]; - const isImageLine = TERMINAL.isImageLine(line); - if (!isImageLine && visibleWidth(line) > width) { - if (process.platform === "win32") { - line = sliceByColumn(line, 0, width, true); - newLines[i] = line; - } else { - // Log all lines to crash file for debugging - const crashLogPath = path.join(os.homedir(), ".omp", "agent", "omp-crash.log"); - const crashData = [ - `Crash at ${new Date().toISOString()}`, - `Terminal width: ${width}`, - `Line ${i} visible width: ${visibleWidth(line)}`, - "", - "=== All rendered lines ===", - ...newLines.map((l, idx) => `[${idx}] (w=${visibleWidth(l)}) ${l}`), - "", - ].join("\n"); - try { - fs.mkdirSync(path.dirname(crashLogPath), { recursive: true }); - fs.writeFileSync(crashLogPath, crashData); - } catch { - // Ignore - crash log is best-effort - } + const line = newLines[i]; + const isImage = TERMINAL.isImageLine(line); + if (!isImage && visibleWidth(line) > width) { + // Log all lines to crash file for debugging + const crashLogPath = path.join(os.homedir(), ".pi", "agent", "pi-crash.log"); + const crashData = [ + `Crash at ${new Date().toISOString()}`, + `Terminal width: ${width}`, + `Line ${i} visible width: ${visibleWidth(line)}`, + "", + "=== All rendered lines ===", + ...newLines.map((l, idx) => `[${idx}] (w=${visibleWidth(l)}) ${l}`), + "", + ].join("\n"); + fs.mkdirSync(path.dirname(crashLogPath), { recursive: true }); + fs.writeFileSync(crashLogPath, crashData); - // Clean up terminal state before throwing - this.stop(); + // Clean up terminal state before throwing + this.stop(); - const errorMsg = [ - `Rendered line ${i} exceeds terminal width (${visibleWidth(line)} > ${width}).`, - "", - "This is likely caused by a custom TUI component not truncating its output.", - "Use visibleWidth() to measure and truncateToWidth() to truncate lines.", - "", - `Debug log written to: ${crashLogPath}`, - ].join("\n"); - throw new Error(errorMsg); - } + const errorMsg = [ + `Rendered line ${i} exceeds terminal width (${visibleWidth(line)} > ${width}).`, + "", + "This is likely caused by a custom TUI component not truncating its output.", + "Use visibleWidth() to measure and truncateToWidth() to truncate lines.", + "", + `Debug log written to: ${crashLogPath}`, + ].join("\n"); + throw new Error(errorMsg); } buffer += line; } @@ -1223,12 +1065,41 @@ export class TUI extends Container { buffer += "\x1b[?2026l"; // End synchronized output + if (process.env.PI_TUI_DEBUG === "1") { + const debugDir = "/tmp/tui"; + fs.mkdirSync(debugDir, { recursive: true }); + const debugPath = path.join(debugDir, `render-${Date.now()}-${Math.random().toString(36).slice(2)}.log`); + const debugData = [ + `firstChanged: ${firstChanged}`, + `viewportTop: ${viewportTop}`, + `cursorRow: ${this.cursorRow}`, + `height: ${height}`, + `lineDiff: ${lineDiff}`, + `hardwareCursorRow: ${hardwareCursorRow}`, + `renderEnd: ${renderEnd}`, + `finalCursorRow: ${finalCursorRow}`, + `cursorPos: ${JSON.stringify(cursorPos)}`, + `newLines.length: ${newLines.length}`, + `previousLines.length: ${this.previousLines.length}`, + "", + "=== newLines ===", + JSON.stringify(newLines, null, 2), + "", + "=== previousLines ===", + JSON.stringify(this.previousLines, null, 2), + "", + "=== buffer ===", + JSON.stringify(buffer), + ].join("\n"); + fs.writeFileSync(debugPath, debugData); + } + // Write entire buffer at once this.terminal.write(buffer); // Track cursor position for next render - // cursorRow represents end-of-content for viewport calculations, - // hardwareCursorRow tracks actual cursor position (may move to cursorPos below) + // cursorRow tracks end of content (for viewport calculation) + // hardwareCursorRow tracks actual terminal cursor position (for movement) this.cursorRow = Math.max(0, newLines.length - 1); this.hardwareCursorRow = finalCursorRow; // Track terminal's working area (grows but doesn't shrink unless cleared) @@ -1241,4 +1112,42 @@ export class TUI extends Container { this.previousLines = newLines; this.previousWidth = width; } + + /** + * Position the hardware cursor for IME candidate window. + * @param cursorPos The cursor position extracted from rendered output, or null + * @param totalLines Total number of rendered lines + */ + private positionHardwareCursor(cursorPos: { row: number; col: number } | null, totalLines: number): void { + if (!cursorPos || totalLines <= 0) { + this.terminal.hideCursor(); + return; + } + + // Clamp cursor position to valid range + const targetRow = Math.max(0, Math.min(cursorPos.row, totalLines - 1)); + const targetCol = Math.max(0, cursorPos.col); + + // Move cursor from current position to target + const rowDelta = targetRow - this.hardwareCursorRow; + let buffer = ""; + if (rowDelta > 0) { + buffer += `\x1b[${rowDelta}B`; // Move down + } else if (rowDelta < 0) { + buffer += `\x1b[${-rowDelta}A`; // Move up + } + // Move to absolute column (1-indexed) + buffer += `\x1b[${targetCol + 1}G`; + + if (buffer) { + this.terminal.write(buffer); + } + + this.hardwareCursorRow = targetRow; + if (this.showHardwareCursor) { + this.terminal.showCursor(); + } else { + this.terminal.hideCursor(); + } + } } diff --git a/packages/tui/src/utils.ts b/packages/tui/src/utils.ts index d6ad80661..ab83b7714 100644 --- a/packages/tui/src/utils.ts +++ b/packages/tui/src/utils.ts @@ -5,6 +5,13 @@ export { Ellipsis, extractSegments, sliceWithWidth, truncateToWidth, wrapTextWit // Pre-allocated space buffer for padding const SPACE_BUFFER = " ".repeat(512); +/* + * Replace tabs with 3 spaces for consistent rendering. + */ +export function replaceTabs(text: string): string { + return text.replaceAll("\t", " "); +} + /** * Returns a string of n spaces. Uses a pre-allocated buffer for efficiency. */ diff --git a/packages/tui/test/virtual-terminal.ts b/packages/tui/test/virtual-terminal.ts index f1ca7e78f..97063c56d 100644 --- a/packages/tui/test/virtual-terminal.ts +++ b/packages/tui/test/virtual-terminal.ts @@ -195,15 +195,4 @@ export class VirtualTerminal implements Terminal { reset(): void { this.xterm.reset(); } - - /** - * Get cursor position - */ - getCursorPosition(): { x: number; y: number } { - const buffer = this.xterm.buffer.active; - return { - x: buffer.cursorX, - y: buffer.cursorY, - }; - } }