From a9d18d7e5793edb55a45282eb181685440e4ccf5 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 03:44:59 +1100 Subject: [PATCH 01/15] Add Screen interface, StdoutScreen, and MockScreen test infrastructure (#146) --- src/Screen.ts | 26 ++++++ test/MockScreen.ts | 121 ++++++++++++++++++++++++++++ test/mock-screen.spec.ts | 166 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 313 insertions(+) create mode 100644 src/Screen.ts create mode 100644 test/MockScreen.ts create mode 100644 test/mock-screen.spec.ts diff --git a/src/Screen.ts b/src/Screen.ts new file mode 100644 index 0000000..f658a15 --- /dev/null +++ b/src/Screen.ts @@ -0,0 +1,26 @@ +export interface Screen { + readonly rows: number; + readonly columns: number; + write(data: string): void; + onResize(cb: (columns: number, rows: number) => void): () => void; +} + +export class StdoutScreen implements Screen { + public get rows(): number { + return process.stdout.rows ?? 24; + } + + public get columns(): number { + return process.stdout.columns ?? 80; + } + + public write(data: string): void { + process.stdout.write(data); + } + + public onResize(cb: (columns: number, rows: number) => void): () => void { + const handler = () => cb(process.stdout.columns ?? 80, process.stdout.rows ?? 24); + process.stdout.on('resize', handler); + return () => process.stdout.off('resize', handler); + } +} diff --git a/test/MockScreen.ts b/test/MockScreen.ts new file mode 100644 index 0000000..982533e --- /dev/null +++ b/test/MockScreen.ts @@ -0,0 +1,121 @@ +import type { Screen } from '../src/Screen.js'; + +export class MockScreen implements Screen { + public readonly cells: string[][]; + public cursorRow = 0; + public cursorCol = 0; + public scrollbackViolations = 0; + + public constructor( + public readonly columns: number, + public readonly rows: number, + ) { + this.cells = Array.from({ length: rows }, () => new Array(columns).fill('')); + } + + public write(data: string): void { + let i = 0; + while (i < data.length) { + const ch = data[i]; + if (ch === '\x1B') { + i++; + if (i < data.length && data[i] === '[') { + i++; + let param = ''; + while (i < data.length && !/[A-Za-z]/.test(data[i] as string)) { + param += data[i]; + i++; + } + if (i < data.length) { + this.handleCsi(param, data[i] as string); + i++; + } + } else { + i++; + } + } else if (ch === '\r') { + this.cursorCol = 0; + i++; + } else if (ch === '\n') { + this.cursorCol = 0; + if (this.cursorRow === this.rows - 1) { + this.scrollbackViolations++; + this.cells.shift(); + this.cells.push(new Array(this.columns).fill('')); + } else { + this.cursorRow++; + } + i++; + } else { + this.cells[this.cursorRow][this.cursorCol] = ch; + this.cursorCol++; + if (this.cursorCol >= this.columns) { + this.cursorCol = 0; + if (this.cursorRow < this.rows - 1) { + this.cursorRow++; + } else { + this.scrollbackViolations++; + this.cells.shift(); + this.cells.push(new Array(this.columns).fill('')); + } + } + i++; + } + } + } + + private handleCsi(param: string, final: string): void { + switch (final) { + case 'A': { + const n = parseInt(param || '1', 10); + this.cursorRow = Math.max(0, this.cursorRow - n); + break; + } + case 'G': { + const n = parseInt(param || '1', 10); + this.cursorCol = Math.min(this.columns - 1, Math.max(0, n - 1)); + break; + } + case 'K': { + if (param === '2') { + for (let c = 0; c < this.columns; c++) { + this.cells[this.cursorRow][c] = ''; + } + } + break; + } + case 'J': { + for (let r = this.cursorRow; r < this.rows; r++) { + for (let c = 0; c < this.columns; c++) { + this.cells[r][c] = ''; + } + } + break; + } + default: + break; + } + } + + public assertNoScrollbackViolations(): void { + if (this.scrollbackViolations > 0) { + throw new Error(`MockScreen: ${this.scrollbackViolations} scrollback violation(s) detected`); + } + } + + public getRow(r: number): string { + const row = this.cells[r]; + let lastNonEmpty = -1; + for (let c = row.length - 1; c >= 0; c--) { + if (row[c] !== '') { + lastNonEmpty = c; + break; + } + } + return row.slice(0, lastNonEmpty + 1).join(''); + } + + public onResize(_cb: (columns: number, rows: number) => void): () => void { + return () => {}; + } +} diff --git a/test/mock-screen.spec.ts b/test/mock-screen.spec.ts new file mode 100644 index 0000000..6dd4e7a --- /dev/null +++ b/test/mock-screen.spec.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from 'vitest'; +import { MockScreen } from './MockScreen.js'; + +const ESC = '\x1B['; +const cursorUp = (n: number) => `${ESC}${n}A`; +const cursorTo = (col: number) => `${ESC}${col + 1}G`; +const clearLine = `${ESC}2K`; + +describe('MockScreen', () => { + it('writes characters at correct cursor position and advances cursor', () => { + const screen = new MockScreen(80, 24); + screen.write('AB'); + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(2); + expect(screen.getRow(0)).toBe('AB'); + }); + + it('writes characters at offset cursor position', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n'); // row 2 + screen.write('XY'); + expect(screen.cursorRow).toBe(2); + expect(screen.cursorCol).toBe(2); + expect(screen.getRow(2)).toBe('XY'); + expect(screen.getRow(0)).toBe(''); + }); + + it('cursorUp moves cursor up N rows', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n\n'); // row 3 + screen.write(cursorUp(2)); + expect(screen.cursorRow).toBe(1); + }); + + it('cursorUp clamped at row 0', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n'); // row 2 + screen.write(cursorUp(10)); + expect(screen.cursorRow).toBe(0); + }); + + it('cursorUp by 1 moves up one row', () => { + const screen = new MockScreen(80, 24); + screen.write('\n'); // row 1 + screen.write(cursorUp(1)); + expect(screen.cursorRow).toBe(0); + }); + + it('cursorTo moves cursor to specified column', () => { + const screen = new MockScreen(80, 24); + screen.write(cursorTo(10)); + expect(screen.cursorCol).toBe(10); + }); + + it('cursorTo column 0 moves to first column', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + screen.write(cursorTo(0)); + expect(screen.cursorCol).toBe(0); + }); + + it('clearLine clears the current row', () => { + const screen = new MockScreen(80, 24); + screen.write('hello world'); + screen.write(clearLine); + expect(screen.getRow(0)).toBe(''); + }); + + it('clearLine does not affect other rows', () => { + const screen = new MockScreen(80, 24); + screen.write('row0'); + screen.write('\n'); + screen.write('row1'); + screen.write('\r'); // move to col 0 on row 1 + screen.write(cursorUp(1)); // back to row 0 + screen.write(clearLine); + expect(screen.getRow(0)).toBe(''); + expect(screen.getRow(1)).toBe('row1'); + }); + + it('\\n above bottom row moves cursor down, no violation', () => { + const screen = new MockScreen(80, 3); + screen.write('\n'); // row 1 + expect(screen.cursorRow).toBe(1); + expect(screen.scrollbackViolations).toBe(0); + }); + + it('\\n at bottom row increments scrollbackViolations', () => { + const screen = new MockScreen(80, 3); + screen.write('\n\n'); // row 2 (last row for 3-row screen) + expect(screen.cursorRow).toBe(2); + screen.write('\n'); + expect(screen.scrollbackViolations).toBe(1); + }); + + it('\\n at bottom row shifts rows up and clears new bottom row', () => { + const screen = new MockScreen(80, 3); + screen.write('A'); + screen.write('\n'); + screen.write('B'); + screen.write('\n'); + screen.write('C'); + // rows: A, B, C (cursorRow=2) + screen.write('\n'); + // scroll: row0(A) lost, row1(B)->row0, row2(C)->row1, new empty row2 + expect(screen.getRow(0)).toBe('B'); + expect(screen.getRow(1)).toBe('C'); + expect(screen.getRow(2)).toBe(''); + expect(screen.scrollbackViolations).toBe(1); + }); + + it('assertNoScrollbackViolations passes when violations = 0', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + expect(() => screen.assertNoScrollbackViolations()).not.toThrow(); + }); + + it('assertNoScrollbackViolations throws when violations > 0', () => { + const screen = new MockScreen(80, 1); + screen.write('\n'); + expect(() => screen.assertNoScrollbackViolations()).toThrow(); + }); + + it('writing fills a row and wraps to the next row', () => { + const screen = new MockScreen(5, 24); + screen.write('ABCDE'); // 5 chars fills 5-col row, wraps + expect(screen.cursorRow).toBe(1); + expect(screen.cursorCol).toBe(0); + expect(screen.getRow(0)).toBe('ABCDE'); + }); + + it('writing past end of last row causes scrollback violation', () => { + const screen = new MockScreen(5, 2); + screen.write('\n'); // row 1 (last row for 2-row screen) + screen.write('ABCDE'); // fills last row, wrap -> scrollback violation + expect(screen.scrollbackViolations).toBe(1); + }); + + it('\\r moves cursor to column 0', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + screen.write('\r'); + expect(screen.cursorCol).toBe(0); + expect(screen.cursorRow).toBe(0); + }); + + it('\\r followed by write overwrites from column 0', () => { + const screen = new MockScreen(80, 24); + screen.write('AAAAA'); + screen.write('\r'); + screen.write('BB'); + expect(screen.getRow(0)).toBe('BBAAA'); + }); + + it('getRow trims trailing empty cells', () => { + const screen = new MockScreen(80, 24); + screen.write('hi'); + expect(screen.getRow(0)).toBe('hi'); + expect(screen.getRow(0).length).toBe(2); + }); + + it('getRow returns empty string for blank row', () => { + const screen = new MockScreen(80, 24); + expect(screen.getRow(0)).toBe(''); + }); +}); From 936b4c5beb99ef60010bfbcd7d4cf922845d5e28 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 03:46:01 +1100 Subject: [PATCH 02/15] Session end: Stage 1 complete (Screen, MockScreen, tests) --- .claude/CLAUDE.md | 4 ++-- .claude/sessions/2026-03-29.md | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 98d6b2d..7bfbea8 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -72,8 +72,8 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State -Branch: `fix/sticky-zone-viewport-overflow` -In-progress: Nothing. PR #145 (fix #134, sticky zone viewport overflow) open, auto-merge enabled. +Branch: `fix/terminal-rendering-redesign` +In-progress: Stage 1 complete (Screen interface, StdoutScreen, MockScreen, tests). Stage 2 (Layout) next. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index a48a3a8..3cfae16 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -15,3 +15,11 @@ - Decisions: Phase 1 (implementation, type-check, build, test, supervisor verification) was completed before this session. This session handled commit, push, and PR only. - Next: Await PR #145 auto-merge. - Violations: None + +### 03:44 - fix/terminal-rendering-redesign (#146) Stage 1 + +- Did: Created branch `fix/terminal-rendering-redesign`. Built `Screen` interface and `StdoutScreen` in `src/Screen.ts`. Built `MockScreen` test double in `test/MockScreen.ts` with character-by-character ANSI parser (cursor movement, clearLine, clearDown, scrollback violation counting). Wrote 20 tests in `test/mock-screen.spec.ts`. All pass. +- Files: `src/Screen.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts` +- Decisions: `MockScreen` placed in `test/` per spec (test double, not production code). Biome `useConsistentMemberAccessibility` required explicit `public` on all members (unsafe fix, done manually). Wrap at last column on last row treated as scrollback violation (matches real terminal scroll behavior). +- Next: Stage 2 (Layout pure function). +- Violations: None From d030949aecd6ebdc25431fcef01626b12af08f07 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 03:51:57 +1100 Subject: [PATCH 03/15] Session start: Stage 2 complete (Layout function, tests) --- src/Layout.ts | 93 ++++++++++++++++++++++ test/layout.spec.ts | 189 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 src/Layout.ts create mode 100644 test/layout.spec.ts diff --git a/src/Layout.ts b/src/Layout.ts new file mode 100644 index 0000000..94b16f7 --- /dev/null +++ b/src/Layout.ts @@ -0,0 +1,93 @@ +import stringWidth from 'string-width'; +import type { EditorRender } from './renderer.js'; + +/** + * Output from an existing builder (status, attachment, preview). + * `rows` are logical lines as the builder produces them (may be wider than columns). + * `height` is the visual height in terminal rows (accounts for wrapping). + */ +export interface BuiltComponent { + rows: string[]; + height: number; +} + +export interface LayoutInput { + editor: EditorRender; + status: BuiltComponent | null; + attachments: BuiltComponent | null; + preview: BuiltComponent | null; + question: BuiltComponent | null; + columns: number; +} + +/** + * Layout output. `buffer` contains one entry per visual (terminal) row. + * Layout is responsible for wrapping: a logical line wider than `columns` + * becomes multiple buffer entries. + */ +export interface LayoutResult { + buffer: string[]; + cursorRow: number; + cursorCol: number; + editorStartRow: number; +} + +/** + * Splits a logical line into visual rows by wrapping at `columns` visual width. + * Returns at least one entry (empty string for empty input). + */ +function wrapLine(line: string, columns: number): string[] { + if (stringWidth(line) <= columns) { + return [line]; + } + const segments: string[] = []; + let current = ''; + let currentWidth = 0; + for (const char of line) { + const cw = stringWidth(char); + if (currentWidth + cw > columns) { + segments.push(current); + current = char; + currentWidth = cw; + } else { + current += char; + currentWidth += cw; + } + } + if (current.length > 0 || segments.length === 0) { + segments.push(current); + } + return segments; +} + +/** + * Pure layout function. Takes all UI components and returns an unbounded + * buffer of visual rows with cursor position metadata. + * + * Buffer order (top to bottom): question, status, attachments, preview, editor. + */ +export function layout(input: LayoutInput): LayoutResult { + const { editor, status, attachments, preview, question, columns } = input; + const buffer: string[] = []; + + for (const component of [question, status, attachments, preview]) { + if (component !== null) { + for (const row of component.rows) { + buffer.push(...wrapLine(row, columns)); + } + } + } + + const editorStartRow = buffer.length; + + for (const line of editor.lines) { + buffer.push(...wrapLine(line, columns)); + } + + return { + buffer, + cursorRow: editorStartRow + editor.cursorRow, + cursorCol: editor.cursorCol, + editorStartRow, + }; +} diff --git a/test/layout.spec.ts b/test/layout.spec.ts new file mode 100644 index 0000000..1045984 --- /dev/null +++ b/test/layout.spec.ts @@ -0,0 +1,189 @@ +import { describe, expect, it } from 'vitest'; +import type { BuiltComponent, LayoutInput } from '../src/Layout.js'; +import { layout } from '../src/Layout.js'; + +function makeEditor(lines: string[], cursorRow = 0, cursorCol = 0) { + return { lines, cursorRow, cursorCol }; +} + +function component(rows: string[], height: number): BuiltComponent { + return { rows, height }; +} + +describe('layout', () => { + it('editor only: 5 single-row lines produce buffer of 5 rows', () => { + const input: LayoutInput = { + editor: makeEditor(['line 1', 'line 2', 'line 3', 'line 4', 'line 5']), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(5); + expect(result.editorStartRow).toBe(0); + expect(result.cursorRow).toBe(0); + expect(result.cursorCol).toBe(0); + }); + + it('status + editor: status row before editor, editorStartRow = 1', () => { + const input: LayoutInput = { + editor: makeEditor(['hello']), + status: component(['status line'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('status line'); + expect(result.buffer[1]).toBe('hello'); + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(1); + }); + + it('all components present: buffer order is question, status, attachment, preview, editor', () => { + const input: LayoutInput = { + editor: makeEditor(['editor']), + status: component(['status'], 1), + attachments: component(['attachment'], 1), + preview: component(['preview'], 1), + question: component(['question'], 1), + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('question'); + expect(result.buffer[1]).toBe('status'); + expect(result.buffer[2]).toBe('attachment'); + expect(result.buffer[3]).toBe('preview'); + expect(result.buffer[4]).toBe('editor'); + expect(result.editorStartRow).toBe(4); + expect(result.cursorRow).toBe(4); + }); + + it('null components skipped: no empty rows in buffer', () => { + const input: LayoutInput = { + editor: makeEditor(['editor']), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(1); + expect(result.buffer[0]).toBe('editor'); + }); + + it('long editor line wraps: 200 chars at 80 columns = 3 buffer rows', () => { + const longLine = 'A'.repeat(200); + const input: LayoutInput = { + editor: makeEditor([longLine], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(3); + expect(result.buffer[0]).toBe('A'.repeat(80)); + expect(result.buffer[1]).toBe('A'.repeat(80)); + expect(result.buffer[2]).toBe('A'.repeat(40)); + }); + + it('50 editor lines: buffer has 50 rows, cursorRow accounts for non-editor rows', () => { + const lines = Array.from({ length: 50 }, (_, i) => `line ${i}`); + const input: LayoutInput = { + editor: makeEditor(lines, 25, 0), + status: component(['status'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(51); // 1 status + 50 editor + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(26); // 1 (status) + 25 (editor cursor) + }); + + it('cursorCol passes through from EditorRender.cursorCol', () => { + const input: LayoutInput = { + editor: makeEditor(['hello world'], 0, 5), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.cursorCol).toBe(5); + }); + + it('editorStartRow correct with two non-editor components', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: component(['s'], 1), + attachments: component(['a'], 1), + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.editorStartRow).toBe(2); + }); + + it('editorStartRow correct with question and preview but no status or attachment', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: null, + attachments: null, + preview: component(['p'], 1), + question: component(['q'], 1), + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('q'); + expect(result.buffer[1]).toBe('p'); + expect(result.editorStartRow).toBe(2); + }); + + it('multi-row component contributes multiple buffer entries', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: null, + attachments: null, + preview: component(['preview line 1', 'preview line 2', 'preview line 3'], 3), + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('preview line 1'); + expect(result.buffer[1]).toBe('preview line 2'); + expect(result.buffer[2]).toBe('preview line 3'); + expect(result.editorStartRow).toBe(3); + expect(result.cursorRow).toBe(3); + }); + + it('editor cursor on wrapped line: cursorRow offset by editorStartRow', () => { + // Editor has 1 wrapped line (160 chars = 2 visual rows at 80 cols) + // cursor is on visual row 1 of the editor (second wrap segment) + // with 1 status row above, buffer cursorRow = 1 (status) + 1 (visual row) = 2 + const wrappedLine = 'B'.repeat(160); + const input: LayoutInput = { + editor: makeEditor([wrappedLine], 1, 10), + status: component(['status'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(3); // 1 status + 2 editor rows + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(2); // 1 + 1 + expect(result.cursorCol).toBe(10); + }); +}); From c30a948f088a94411c8e551fd82c2c622547065b Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 03:53:22 +1100 Subject: [PATCH 04/15] Session end: Stage 2 complete (Layout, tests) --- .claude/CLAUDE.md | 2 +- .claude/sessions/2026-03-29.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 7bfbea8..c90bb6b 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: Stage 1 complete (Screen interface, StdoutScreen, MockScreen, tests). Stage 2 (Layout) next. +In-progress: Stage 2 complete (Layout function, tests). Stage 3 (Viewport) next. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 3cfae16..963535d 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -23,3 +23,11 @@ - Decisions: `MockScreen` placed in `test/` per spec (test double, not production code). Biome `useConsistentMemberAccessibility` required explicit `public` on all members (unsafe fix, done manually). Wrap at last column on last row treated as scrollback violation (matches real terminal scroll behavior). - Next: Stage 2 (Layout pure function). - Violations: None + +### 03:51 - fix/terminal-rendering-redesign (#146) Stage 2 + +- Did: Implemented `layout()` pure function in `src/Layout.ts` with `BuiltComponent`, `LayoutInput`, `LayoutResult` interfaces and a `wrapLine()` helper. Wrote 11 tests in `test/layout.spec.ts` covering all cases from the spec. All 140 tests pass. +- Files: `src/Layout.ts`, `test/layout.spec.ts` +- Decisions: `wrapLine` uses `stringWidth` char-by-char for correct visual width splitting. ANSI-escaped BuiltComponent rows that don't exceed `columns` pass through as-is (typical for status lines); ANSI-aware splitting for wider strings deferred to Stage 5 if needed. +- Next: Stage 3 (Viewport stateful class). +- Violations: None From 86f4aa288721500ebd509aad37a936554895e476 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 03:59:06 +1100 Subject: [PATCH 05/15] Add Viewport class with scroll offset and cursor chasing --- src/Viewport.ts | 31 +++++++++++ test/viewport.spec.ts | 125 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 src/Viewport.ts create mode 100644 test/viewport.spec.ts diff --git a/src/Viewport.ts b/src/Viewport.ts new file mode 100644 index 0000000..afaef57 --- /dev/null +++ b/src/Viewport.ts @@ -0,0 +1,31 @@ +export interface ViewportResult { + rows: string[]; + visibleCursorRow: number; + visibleCursorCol: number; +} + +export class Viewport { + private scrollOffset = 0; + + public resolve(buffer: string[], screenRows: number, cursorRow: number, cursorCol: number): ViewportResult { + // Cap scrollOffset after a potential resize + this.scrollOffset = Math.min(this.scrollOffset, Math.max(0, buffer.length - screenRows)); + + // Cursor chasing + if (cursorRow < this.scrollOffset) { + this.scrollOffset = cursorRow; + } else if (cursorRow >= this.scrollOffset + screenRows) { + this.scrollOffset = cursorRow - screenRows + 1; + } + + const slice = buffer.slice(this.scrollOffset, this.scrollOffset + screenRows); + const padding = screenRows - slice.length; + const rows = padding > 0 ? [...slice, ...Array(padding).fill('')] : slice; + + return { + rows, + visibleCursorRow: cursorRow - this.scrollOffset, + visibleCursorCol: cursorCol, + }; + } +} diff --git a/test/viewport.spec.ts b/test/viewport.spec.ts new file mode 100644 index 0000000..1250934 --- /dev/null +++ b/test/viewport.spec.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from 'vitest'; +import { Viewport } from '../src/Viewport.js'; + +describe('Viewport', () => { + it('buffer shorter than screen: returns screenRows entries (content + padding)', () => { + const vp = new Viewport(); + const buffer = ['a', 'b', 'c', 'd', 'e']; + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + expect(result.rows[0]).toBe('a'); + expect(result.rows[4]).toBe('e'); + expect(result.rows[5]).toBe(''); + expect(result.rows[9]).toBe(''); + }); + + it('buffer equals screen: returns all rows, no padding', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 10 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + expect(result.rows[0]).toBe('row 0'); + expect(result.rows[9]).toBe('row 9'); + }); + + it('buffer taller than screen: returns exactly screenRows rows', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + }); + + it('cursor at row 0: scrollOffset = 0, visibleCursorRow = 0', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 5 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 3); + expect(result.visibleCursorRow).toBe(0); + expect(result.visibleCursorCol).toBe(3); + }); + + it('cursor at row 45 (buffer 50, screen 10): scrollOffset adjusts, visibleCursorRow within 0-9', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 45, 0); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(9); + // cursor at 45, screen 10 => scrollOffset = 45 - 10 + 1 = 36, visibleCursorRow = 45 - 36 = 9 + expect(result.visibleCursorRow).toBe(9); + expect(result.rows[9]).toBe('row 45'); + }); + + it('cursor in stable range: scrollOffset does not change when cursor is already visible', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call places cursor at row 45 (scrollOffset = 36) + vp.resolve(buffer, 10, 45, 0); + // Second call: cursor moves to row 40 (still in 36..45 range) + const result = vp.resolve(buffer, 10, 40, 0); + // scrollOffset stays 36, visibleCursorRow = 40 - 36 = 4 + expect(result.visibleCursorRow).toBe(4); + expect(result.rows[4]).toBe('row 40'); + }); + + it('cursor chasing up: cursor moves above viewport, scrollOffset snaps to cursorRow', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call: cursor at 45, scrollOffset = 36 + vp.resolve(buffer, 10, 45, 0); + // Second call: cursor jumps to row 5 (below scrollOffset=36) + const result = vp.resolve(buffer, 10, 5, 0); + // scrollOffset snaps to 5, visibleCursorRow = 0 + expect(result.visibleCursorRow).toBe(0); + expect(result.rows[0]).toBe('row 5'); + }); + + it('cursor chasing down: scrollOffset = cursorRow - screenRows + 1', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call: cursor at 0, scrollOffset = 0 + vp.resolve(buffer, 10, 0, 0); + // Second call: cursor jumps to row 20 (beyond 0 + 10 - 1 = 9) + const result = vp.resolve(buffer, 10, 20, 0); + // scrollOffset = 20 - 10 + 1 = 11, visibleCursorRow = 20 - 11 = 9 + expect(result.visibleCursorRow).toBe(9); + expect(result.rows[9]).toBe('row 20'); + }); + + it('resize shrink (24 to 10): scrollOffset capped, cursor still visible', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call at screen height 24, cursor at 30 + // scrollOffset = 30 - 24 + 1 = 7 + vp.resolve(buffer, 24, 30, 0); + // Resize to 10: scrollOffset capped at max(0, 50 - 10) = 40, then cursor chasing + // cursor 30 < scrollOffset(after cap)... let's trace: + // After cap: scrollOffset = min(7, max(0, 50 - 10)) = min(7, 40) = 7 + // Then cursor chasing: cursorRow(30) >= 7 + 10 = 17 => scrollOffset = 30 - 10 + 1 = 21 + // visibleCursorRow = 30 - 21 = 9 + const result = vp.resolve(buffer, 10, 30, 0); + expect(result.rows.length).toBe(10); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(9); + }); + + it('resize grow (10 to 24): more content visible, scrollOffset does not increase', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call at screen 10, cursor at 45 => scrollOffset = 36 + vp.resolve(buffer, 10, 45, 0); + // Grow to 24, cursor still at 45 + // cap: scrollOffset = min(36, max(0, 50 - 24)) = min(36, 26) = 26 + // cursor chasing: 45 >= 26 + 24 = 50? No. 45 < 26? No. => stays 26 + // visibleCursorRow = 45 - 26 = 19 + const result = vp.resolve(buffer, 24, 45, 0); + expect(result.rows.length).toBe(24); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(23); + }); + + it('visibleCursorCol always equals input cursorCol', () => { + const vp = new Viewport(); + const buffer = ['a', 'b', 'c']; + const result = vp.resolve(buffer, 10, 1, 42); + expect(result.visibleCursorCol).toBe(42); + }); +}); From dc70bf6a4bcb775a1f51a5be51522037c6b78417 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 04:01:30 +1100 Subject: [PATCH 06/15] Session end: Stage 3 complete (Viewport, tests) --- .claude/CLAUDE.md | 2 +- .claude/sessions/2026-03-29.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index c90bb6b..52b34f0 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: Stage 2 complete (Layout function, tests). Stage 3 (Viewport) next. +In-progress: Stage 3 complete (Viewport class, tests). Stage 4 (Renderer) next. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 963535d..a12b3c5 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -31,3 +31,11 @@ - Decisions: `wrapLine` uses `stringWidth` char-by-char for correct visual width splitting. ANSI-escaped BuiltComponent rows that don't exceed `columns` pass through as-is (typical for status lines); ANSI-aware splitting for wider strings deferred to Stage 5 if needed. - Next: Stage 3 (Viewport stateful class). - Violations: None + +### 03:54 - fix/terminal-rendering-redesign (#146) Stage 3 + +- Did: Implemented `Viewport` class in `src/Viewport.ts` with `scrollOffset` state, cursor chasing (up/down), and resize capping. Wrote 11 tests in `test/viewport.spec.ts` covering all spec cases. All 151 tests pass. Committed and pushed. +- Files: `src/Viewport.ts`, `test/viewport.spec.ts` +- Decisions: Biome `useConsistentMemberAccessibility` required explicit `public` on `resolve()` (unsafe fix, applied manually, consistent with Stage 1 pattern). +- Next: Stage 4 (Renderer class). +- Violations: None From 35cb4d66871d1fa5f42d78ce85e7ebc32ee1bddb Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 04:33:49 +1100 Subject: [PATCH 07/15] Add Renderer class with scrollback-safe zone rendering --- src/TerminalRenderer.ts | 60 +++++++++++++++++++++++++ test/renderer.spec.ts | 98 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 src/TerminalRenderer.ts create mode 100644 test/renderer.spec.ts diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts new file mode 100644 index 0000000..97f7772 --- /dev/null +++ b/src/TerminalRenderer.ts @@ -0,0 +1,60 @@ +import type { Screen } from './Screen.js'; +import type { ViewportResult } from './Viewport.js'; + +const ESC = '\x1B['; +const cursorUp = (n: number) => (n > 0 ? `${ESC}${n}A` : ''); +const cursorTo = (col: number) => `${ESC}${col + 1}G`; +const clearLine = `${ESC}2K`; +const clearDown = `${ESC}J`; +const showCursor = `${ESC}?25h`; +const hideCursor = `${ESC}?25l`; +const syncStart = '\x1B[?2026h'; +const syncEnd = '\x1B[?2026l'; + +export class Renderer { + public zoneHeight = 0; + private lastVisibleCursorRow = 0; + private lastFrame: ViewportResult | null = null; + + public constructor(private readonly screen: Screen) {} + + public render(frame: ViewportResult): void { + let out = syncStart + hideCursor; + out += cursorUp(this.lastVisibleCursorRow); + out += this.buildZoneOutput(frame); + out += showCursor + syncEnd; + this.zoneHeight = frame.rows.length; + this.lastVisibleCursorRow = frame.visibleCursorRow; + this.lastFrame = frame; + this.screen.write(out); + } + + public writeHistory(line: string): void { + let out = cursorUp(this.lastVisibleCursorRow); + out += '\r' + line + '\n'; + this.screen.write(out); + if (this.lastFrame !== null) { + this.screen.write(this.buildZoneOutput(this.lastFrame)); + } + } + + private buildZoneOutput(frame: ViewportResult): string { + let out = ''; + // Write all rows except the last, each followed by \n to advance the cursor. + for (let i = 0; i < frame.rows.length - 1; i++) { + out += '\r' + clearLine + frame.rows[i] + '\n'; + } + // clearDown here clears leftover rows from a taller previous frame. + // It must come before the last row write so the last row's content is not erased. + out += clearDown; + // Write the last row without \n to avoid a scrollback violation on a full screen. + const lastRow = frame.rows[frame.rows.length - 1]; + if (lastRow !== undefined) { + out += '\r' + clearLine + lastRow; + } + const rowsFromBottom = frame.rows.length - 1 - frame.visibleCursorRow; + out += cursorUp(rowsFromBottom); + out += cursorTo(frame.visibleCursorCol); + return out; + } +} diff --git a/test/renderer.spec.ts b/test/renderer.spec.ts new file mode 100644 index 0000000..80c694b --- /dev/null +++ b/test/renderer.spec.ts @@ -0,0 +1,98 @@ +import { describe, expect, it } from 'vitest'; +import { Renderer } from '../src/TerminalRenderer.js'; +import type { ViewportResult } from '../src/Viewport.js'; +import { MockScreen } from './MockScreen.js'; + +function makeFrame(rows: string[], visibleCursorRow: number, visibleCursorCol: number): ViewportResult { + return { rows, visibleCursorRow, visibleCursorCol }; +} + +describe('Renderer', () => { + it('single frame on 10-row screen: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['line 0', 'line 1', 'line 2', 'line 3', 'line 4'], 2, 0)); + screen.assertNoScrollbackViolations(); + }); + + it('frame filling entire 10-row screen: exactly 10 rows written, no violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + const rows = Array.from({ length: 10 }, (_, i) => `row ${i}`); + renderer.render(makeFrame(rows, 9, 0)); + screen.assertNoScrollbackViolations(); + for (let i = 0; i < 10; i++) { + expect(screen.getRow(i)).toBe(`row ${i}`); + } + }); + + it('two consecutive frames: no violations, screen shows second frame content', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['a0', 'a1', 'a2', 'a3', 'a4'], 2, 0)); + renderer.render(makeFrame(['b0', 'b1', 'b2', 'b3', 'b4'], 2, 0)); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('b0'); + expect(screen.getRow(4)).toBe('b4'); + }); + + it('shorter frame after taller frame: leftover rows cleared, no violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render( + makeFrame( + Array.from({ length: 8 }, (_, i) => `long${i}`), + 4, + 0, + ), + ); + renderer.render(makeFrame(['short0', 'short1', 'short2'], 1, 0)); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('short0'); + expect(screen.getRow(2)).toBe('short2'); + // Leftover rows from 8-row frame should be cleared + expect(screen.getRow(3)).toBe(''); + expect(screen.getRow(7)).toBe(''); + }); + + it('writeHistory then render: history line above zone, zone re-rendered, no violations', () => { + const screen = new MockScreen(80, 20); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['zone0', 'zone1', 'zone2', 'zone3', 'zone4'], 2, 0)); + renderer.writeHistory('history'); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('history'); + expect(screen.getRow(1)).toBe('zone0'); + expect(screen.getRow(5)).toBe('zone4'); + }); + + it('writeHistory preserves zoneHeight: zoneHeight unchanged after writeHistory', () => { + const screen = new MockScreen(80, 20); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 2, 0)); + const before = renderer.zoneHeight; + renderer.writeHistory('a line'); + expect(renderer.zoneHeight).toBe(before); + }); + + it('multiple writeHistory calls between renders: each pushes zone down, no violations', () => { + const screen = new MockScreen(80, 30); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 2, 0)); + for (let i = 0; i < 10; i++) { + renderer.writeHistory(`history ${i}`); + } + screen.assertNoScrollbackViolations(); + // Zone has drifted 10 rows down from its original position at rows 0-4 + expect(screen.getRow(10)).toBe('z0'); + expect(screen.getRow(14)).toBe('z4'); + }); + + it('cursor position: after render, cursor is at (visibleCursorRow, visibleCursorCol)', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['a', 'b', 'c', 'd', 'e'], 3, 15)); + expect(screen.cursorRow).toBe(3); + expect(screen.cursorCol).toBe(15); + }); +}); From 833a84f18eea26be5b2251f4bc5ff762d8b3b26e Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 04:36:17 +1100 Subject: [PATCH 08/15] Session end: Stage 4 complete (Renderer, tests) --- .claude/CLAUDE.md | 2 +- .claude/sessions/2026-03-29.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 52b34f0..fa30d1a 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: Stage 3 complete (Viewport class, tests). Stage 4 (Renderer) next. +In-progress: Stage 4 complete (Renderer class, tests). Stage 5 (Wire + Replace) next. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index a12b3c5..054813d 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -39,3 +39,11 @@ - Decisions: Biome `useConsistentMemberAccessibility` required explicit `public` on `resolve()` (unsafe fix, applied manually, consistent with Stage 1 pattern). - Next: Stage 4 (Renderer class). - Violations: None + +### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 + +- Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed. +- Files: `src/TerminalRenderer.ts`, `test/renderer.spec.ts` +- Decisions: File named `TerminalRenderer.ts` not `Renderer.ts` because macOS case-insensitive FS treats `Renderer.ts` and `renderer.ts` as the same file (existing `renderer.ts` contains `prepareEditor`). `clearDown` must precede the last row write, not follow it: MockScreen clears the entire current row on `ESC[J`, so issuing clearDown after writing the last row erases it. The fix issues clearDown when cursor is on the last row position before writing content there. +- Next: Stage 5 (Wire + Replace). +- Violations: None From 4b689684b282a6d882105980b2fe18b7bf10b786 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 15:56:52 +1100 Subject: [PATCH 09/15] Fix scrollback corruption when editor exceeds terminal height (#146) --- .claude/CLAUDE.md | 4 +- .claude/sessions/2026-03-29.md | 16 ++ src/TerminalRenderer.ts | 55 ++++++- src/terminal.ts | 242 ++++++++---------------------- test/terminal-integration.spec.ts | 237 +++++++++++++++++++++++++++++ 5 files changed, 369 insertions(+), 185 deletions(-) create mode 100644 test/terminal-integration.spec.ts diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index fa30d1a..078dc3f 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: Stage 4 complete (Renderer class, tests). Stage 5 (Wire + Replace) next. +In-progress: All stages complete. PR created, awaiting merge. @@ -176,7 +176,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she 6. **Context thresholds hardcoded** — 85%/90% tool disable thresholds are not configurable. -7. **Cursor positioning is fragile** — `stickyLineCount` is a single point of truth and failure. Occasional off-by-1 documented but not reliably reproducible. +7. **Cursor positioning via Viewport**: `Viewport.scrollOffset` tracks visible window into layout rows. Off-by-1 errors at screen boundaries are possible but not yet observed post-Stage 5 rewrite. 8. **Null unsets in config merge are subtle** — `"model": null` in local config means "use home config's model", not "set to null". Easy to confuse. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 054813d..989bf21 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -40,6 +40,22 @@ - Next: Stage 4 (Renderer class). - Violations: None +### 05:29 - fix/terminal-rendering-redesign (#146) Stage 5 + +- Did: Wired Layout/Viewport/Renderer pipeline into Terminal class. Replaced `buildSticky()`/`clearStickyZone()` with `renderZone()` pipeline. Added `buildLayoutInput()` that maps existing builders to `BuiltComponent` format. Fixed two bugs found during manual testing: (1) render() wrote padded viewport rows causing scrollback violations on first render (fix: trim trailing empty rows before writing); (2) writeHistory() re-rendered stale `lastFrame` with cleared-question still visible (fix: split into `writeHistoryLine()` + fresh `renderZone()` for current component state). Added 8 integration tests in `test/terminal-integration.spec.ts`. All 167 tests pass. Type-check and build clean. +- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts` +- Decisions: `writeHistoryLine()` added to Renderer to write a history line and reset cursor tracking without triggering a zone re-render; Terminal calls it then calls `renderZone()` separately for a fresh frame. Trailing empty row trim applied in both `render()` and `writeHistory()` paths to prevent padding rows from advancing cursor past screen bottom. No commit yet per Stage 5 instructions (do not commit; await supervisor manual verification). +- Next: Manual verification by Supreme Commander, then commit/push/PR with `Closes #146`. +- Violations: None + +### 15:46 - fix/terminal-rendering-redesign (#146) Commit/Push/PR + +- Did: Committed Stage 5 changes after supervisor manual verification. Pushed branch. Created PR with `Closes #146`. +- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts`, `.claude/CLAUDE.md`, `.claude/sessions/2026-03-29.md` +- Decisions: Session log and CLAUDE.md state updates included in the same commit as the code changes per session protocol. +- Next: Await PR auto-merge. +- Violations: None + ### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 - Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed. diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts index 97f7772..31aaff1 100644 --- a/src/TerminalRenderer.ts +++ b/src/TerminalRenderer.ts @@ -1,3 +1,4 @@ +import stringWidth from 'string-width'; import type { Screen } from './Screen.js'; import type { ViewportResult } from './Viewport.js'; @@ -19,22 +20,68 @@ export class Renderer { public constructor(private readonly screen: Screen) {} public render(frame: ViewportResult): void { + // Trim trailing empty rows from the Viewport-padded frame. Padding is correct + // for Viewport's contract but would cause the Renderer to write screenRows rows + // from wherever the cursor currently is, scrolling the terminal unnecessarily. + let trimEnd = frame.rows.length; + while (trimEnd > 1 && frame.rows[trimEnd - 1] === '') { + trimEnd--; + } + const renderFrame = + trimEnd === frame.rows.length + ? frame + : { + rows: frame.rows.slice(0, trimEnd), + visibleCursorRow: Math.min(frame.visibleCursorRow, trimEnd - 1), + visibleCursorCol: frame.visibleCursorCol, + }; let out = syncStart + hideCursor; out += cursorUp(this.lastVisibleCursorRow); - out += this.buildZoneOutput(frame); + out += this.buildZoneOutput(renderFrame); out += showCursor + syncEnd; - this.zoneHeight = frame.rows.length; - this.lastVisibleCursorRow = frame.visibleCursorRow; + this.zoneHeight = renderFrame.rows.length; + // Track the actual visual cursor row offset from zone top. Rows that are + // exactly `columns` wide wrap the cursor to the next row, so the visual + // offset is greater than the logical row index when any row above the + // cursor wraps. + const wrapAboveCursor = renderFrame.rows.slice(0, renderFrame.visibleCursorRow).reduce((sum, row) => sum + Math.floor(stringWidth(row) / this.screen.columns), 0); + this.lastVisibleCursorRow = renderFrame.visibleCursorRow + wrapAboveCursor; this.lastFrame = frame; this.screen.write(out); } + /** + * Writes a history line above the zone and resets cursor tracking so the + * CALLER can re-render the zone from the current position. Does NOT + * re-render the zone itself. Use this when the caller will supply a fresh + * frame (e.g. Terminal.writeHistory → renderZone). + */ + public writeHistoryLine(line: string): void { + const out = cursorUp(this.lastVisibleCursorRow) + '\r' + clearLine + line + '\n'; + this.lastVisibleCursorRow = 0; + this.screen.write(out); + } + + /** @deprecated Prefer writeHistoryLine + external render for fresh frames. */ public writeHistory(line: string): void { let out = cursorUp(this.lastVisibleCursorRow); out += '\r' + line + '\n'; this.screen.write(out); if (this.lastFrame !== null) { - this.screen.write(this.buildZoneOutput(this.lastFrame)); + // Trim trailing empty rows so the re-rendered zone fits after the zone shifts + // down by one row. Viewport pads frames to screenRows; those empty rows are + // correct for render() but cause scrollback violations here. + let trimEnd = this.lastFrame.rows.length; + while (trimEnd > 1 && this.lastFrame.rows[trimEnd - 1] === '') { + trimEnd--; + } + const trimmedRows = this.lastFrame.rows.slice(0, trimEnd); + const trimmedFrame = { + rows: trimmedRows, + visibleCursorRow: Math.min(this.lastFrame.visibleCursorRow, trimmedRows.length - 1), + visibleCursorCol: this.lastFrame.visibleCursorCol, + }; + this.screen.write(this.buildZoneOutput(trimmedFrame)); } } diff --git a/src/terminal.ts b/src/terminal.ts index f251480..2158e9b 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -5,18 +5,18 @@ import type { AppState } from './AppState.js'; import type { AttachmentStore } from './AttachmentStore.js'; import type { CommandMode } from './CommandMode.js'; import type { EditorState } from './editor.js'; +import type { BuiltComponent, LayoutInput } from './Layout.js'; +import { layout } from './Layout.js'; import { type EditorRender, prepareEditor } from './renderer.js'; +import type { Screen } from './Screen.js'; +import { StdoutScreen } from './Screen.js'; import { StatusLineBuilder } from './StatusLineBuilder.js'; +import { Renderer } from './TerminalRenderer.js'; +import { Viewport } from './Viewport.js'; const TIME_FORMAT = DateTimeFormatter.ofPattern('HH:mm:ss.SSS'); const ESC = '\x1B['; -const cursorUp = (n: number) => (n > 0 ? `${ESC}${n}A` : ''); -const cursorDown = (n: number) => (n > 0 ? `${ESC}${n}B` : ''); -const cursorTo = (col: number) => `${ESC}${col + 1}G`; -const clearLine = `${ESC}2K`; -const clearDown = `${ESC}J`; -const showCursor = `${ESC}?25h`; const hideCursorSeq = `${ESC}?25l`; const resetStyle = `${ESC}0m`; const inverseOn = `${ESC}7m`; @@ -25,13 +25,13 @@ const bel = '\x07'; export class Terminal { private editorContent: EditorRender = { lines: [], cursorRow: 0, cursorCol: 0 }; - private stickyLineCount = 0; - private cursorLinesFromBottom = 0; private cursorHidden = false; - private scrollOffset = 0; private _paused = false; private pauseBuffer: string[] = []; private questionLines: string[] = []; + private readonly screen: Screen; + private readonly viewport: Viewport; + private readonly renderer: Renderer; public sessionId: string | undefined; public modelOverride: string | undefined; @@ -40,7 +40,11 @@ export class Terminal { private drowningThreshold: number | null, private readonly attachmentStore: AttachmentStore, private readonly commandMode: CommandMode, - ) {} + ) { + this.screen = new StdoutScreen(); + this.viewport = new Viewport(); + this.renderer = new Renderer(this.screen); + } public updateConfig(drowningThreshold: number | null): void { this.drowningThreshold = drowningThreshold; @@ -98,19 +102,6 @@ export class Terminal { } } - private clearStickyZone(): string { - if (this.stickyLineCount === 0) { - return ''; - } - let output = ''; - // Move cursor to bottom of sticky zone first, then up to top - output += cursorDown(this.cursorLinesFromBottom); - output += cursorUp(this.stickyLineCount - 1); - output += '\r'; - output += clearDown; - return output; - } - private buildStatusLine(columns: number, allowIdle: boolean): { line: string; screenLines: number } | null { const b = new StatusLineBuilder(); const phase = this.appState.phase; @@ -256,156 +247,58 @@ export class Terminal { return { lines, screenLines }; } - private buildSticky(): string { - const columns = process.stdout.columns || 80; - const terminalRows = process.stdout.rows || 24; - - const attachmentLine = this.buildAttachmentLine(columns, this.commandMode.active); - const statusLine = this.buildStatusLine(columns, !attachmentLine); + private buildLayoutInput(columns: number): LayoutInput { + const attachmentResult = this.buildAttachmentLine(columns, this.commandMode.active); + const statusResult = this.buildStatusLine(columns, !attachmentResult); + const previewResult = this.buildPreviewLines(columns); - // Pre-build each non-editor component into discrete parts (no leading/trailing newlines). + const statusComp: BuiltComponent | null = statusResult ? { rows: [statusResult.line], height: statusResult.screenLines } : null; + let attachComp: BuiltComponent | null = attachmentResult ? { rows: [attachmentResult.line], height: attachmentResult.screenLines } : null; + let previewComp: BuiltComponent | null = previewResult ? { rows: previewResult.lines, height: previewResult.screenLines } : null; - const questionParts: string[] = []; - let questionScreenLines = 0; + let questionHeight = 0; for (const line of this.questionLines) { - questionParts.push(clearLine + line); - questionScreenLines += Math.max(1, Math.ceil(stringWidth(line) / columns)); - } - - let statusPart = ''; - let statusScreenLines = 0; - if (statusLine) { - statusPart = clearLine + statusLine.line; - statusScreenLines = statusLine.screenLines; - } - - let attachmentPart = ''; - let attachmentScreenLines = 0; - if (attachmentLine) { - attachmentPart = clearLine + attachmentLine.line; - attachmentScreenLines = attachmentLine.screenLines; + questionHeight += Math.max(1, Math.ceil(stringWidth(line) / columns)); } + let questionComp: BuiltComponent | null = this.questionLines.length > 0 ? { rows: this.questionLines, height: questionHeight } : null; - const previewParts: string[] = []; - let previewScreenLines = 0; - const preview = this.buildPreviewLines(columns); - if (preview) { - for (const line of preview.lines) { - previewParts.push(clearLine + line); - } - previewScreenLines = preview.screenLines; - } - - // Budget allocation: reserve at least 1 row for the editor. Drop lowest-priority - // components (preview, then attachment, then question) if non-editor content alone - // would consume the entire terminal height. - let nonEditorRows = questionScreenLines + statusScreenLines + attachmentScreenLines + previewScreenLines; + const terminalRows = this.screen.rows; const minEditorRows = 1; + const maxNonEditor = terminalRows - minEditorRows; + let nonEditorRows = (statusComp?.height ?? 0) + (attachComp?.height ?? 0) + (previewComp?.height ?? 0) + (questionComp?.height ?? 0); - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= previewScreenLines; - previewParts.length = 0; - previewScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + nonEditorRows -= previewComp?.height ?? 0; + previewComp = null; } - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= attachmentScreenLines; - attachmentPart = ''; - attachmentScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + nonEditorRows -= attachComp?.height ?? 0; + attachComp = null; } - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= questionScreenLines; - questionParts.length = 0; - questionScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + questionComp = null; } - const availableRows = Math.max(minEditorRows, terminalRows - nonEditorRows); - - // Assemble non-editor output. Preview always uses a leading newline (matches original behaviour). - const topParts = [...questionParts]; - if (statusPart) { - topParts.push(statusPart); - } - if (attachmentPart) { - topParts.push(attachmentPart); - } - - let output = ''; - let hasOutput = false; - for (const part of topParts) { - if (hasOutput) { - output += '\n'; - } - output += part; - hasOutput = true; - } - for (const part of previewParts) { - output += '\n'; - output += part; - } - - // Build a map from logical line index to its starting terminal row within the editor. - const lineStartRow: number[] = []; - let nextStartRow = 0; - for (let i = 0; i < this.editorContent.lines.length; i++) { - lineStartRow.push(nextStartRow); - nextStartRow += Math.max(1, Math.ceil(stringWidth(this.editorContent.lines[i]) / columns)); - } - - // Adjust scrollOffset so the cursor row stays within the visible window. - const cursorRow = this.editorContent.cursorRow; - if (cursorRow < this.scrollOffset) { - this.scrollOffset = cursorRow; - } else if (cursorRow >= this.scrollOffset + availableRows) { - this.scrollOffset = cursorRow - availableRows + 1; - } - - // Cap scrollOffset so content is never scrolled past the end (no empty rows below content). - const maxScrollOffset = Math.max(0, nextStartRow - availableRows); - this.scrollOffset = Math.min(this.scrollOffset, maxScrollOffset); - - // Snap scrollOffset backward to the nearest logical line boundary so we - // never start rendering mid-way through a wrapped logical line. - let snapped = 0; - for (let i = 0; i < lineStartRow.length; i++) { - if (lineStartRow[i] <= this.scrollOffset) { - snapped = lineStartRow[i]; - } else { - break; - } - } - this.scrollOffset = snapped; - - // Render logical lines whose start terminal row falls within the visible window. - let editorScreenLines = 0; - for (let i = 0; i < this.editorContent.lines.length; i++) { - const start = lineStartRow[i]; - const rows = Math.max(1, Math.ceil(stringWidth(this.editorContent.lines[i]) / columns)); - if (start >= this.scrollOffset && start < this.scrollOffset + availableRows) { - output += '\n'; - output += clearLine + this.editorContent.lines[i]; - // Count how many of this line's terminal rows actually fit in the window. - const visibleRows = Math.min(rows, this.scrollOffset + availableRows - start); - editorScreenLines += visibleRows; - } - } - - // Clear any leftover lines from previous render - output += clearDown; + return { + editor: this.editorContent, + status: statusComp, + attachments: attachComp, + preview: previewComp, + question: questionComp, + columns, + }; + } - // Position cursor within the visible editor window. - // cursorRow is the absolute terminal row within the full editor. Subtract - // scrollOffset to get the row within the rendered window. - const visibleCursorRow = cursorRow - this.scrollOffset; - this.cursorLinesFromBottom = editorScreenLines - visibleCursorRow - 1; - if (this.cursorLinesFromBottom > 0) { - output += cursorUp(this.cursorLinesFromBottom); + private renderZone(): void { + const columns = this.screen.columns; + const rows = this.screen.rows; + const input = this.buildLayoutInput(columns); + const result = layout(input); + const frame = this.viewport.resolve(result.buffer, rows, result.cursorRow, result.cursorCol); + this.renderer.render(frame); + if (this.cursorHidden) { + this.screen.write(hideCursorSeq); } - output += cursorTo(this.editorContent.cursorCol % columns); - output += this.cursorHidden ? hideCursorSeq : showCursor; - - this.stickyLineCount = statusScreenLines + attachmentScreenLines + previewScreenLines + questionScreenLines + editorScreenLines; - - return output; } private writeHistory(line: string): void { @@ -413,13 +306,12 @@ export class Terminal { this.pauseBuffer.push(line); return; } - let output = ''; - output += this.clearStickyZone(); - output += line; - output += '\n'; - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + // writeHistoryLine moves to zone top, writes the line, and resets cursor + // tracking. renderZone then re-renders with the CURRENT layout state so the + // zone reflects any changes that happened before this write (e.g. question + // cleared by clearQuestionLines before the history write). + this.renderer.writeHistoryLine(line); + this.renderZone(); } public setQuestionLines(lines: string[]): void { @@ -437,11 +329,7 @@ export class Terminal { if (this.paused) { return; } - let output = ''; - output += this.clearStickyZone(); - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + this.renderZone(); } public log(message: string, ...args: unknown[]): void { @@ -457,24 +345,20 @@ export class Terminal { if (this.paused) { return; } - let output = ''; - output += this.clearStickyZone(); this.editorContent = prepareEditor(editor, prompt); this.cursorHidden = hideCursor; - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + this.renderZone(); } public write(data: string): void { if (this.paused) { return; } - process.stdout.write(data); + this.screen.write(data); } public beep(): void { - process.stdout.write(bel); + this.screen.write(bel); } public error(message: string): void { diff --git a/test/terminal-integration.spec.ts b/test/terminal-integration.spec.ts new file mode 100644 index 0000000..91ac36e --- /dev/null +++ b/test/terminal-integration.spec.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from 'vitest'; +import type { BuiltComponent, LayoutInput } from '../src/Layout.js'; +import { layout } from '../src/Layout.js'; +import type { EditorRender } from '../src/renderer.js'; +import { Renderer } from '../src/TerminalRenderer.js'; +import { Viewport } from '../src/Viewport.js'; +import { MockScreen } from './MockScreen.js'; + +function makeEditorRender(lineCount: number, cursorRow = 0, cursorCol = 0): EditorRender { + const lines = Array.from({ length: lineCount }, (_, i) => `line ${i}`); + return { lines, cursorRow, cursorCol }; +} + +function makeComponent(rows: string[]): BuiltComponent { + return { rows, height: rows.length }; +} + +function runPipeline(screen: MockScreen, viewport: Viewport, renderer: Renderer, input: LayoutInput): void { + const result = layout(input); + const frame = viewport.resolve(result.buffer, screen.rows, result.cursorRow, result.cursorCol); + renderer.render(frame); +} + +describe('Terminal integration', () => { + it('full cycle: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(3, 1, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('50-line editor on 10-row screen: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(50, 25, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('resize from 24 to 10 rows: viewport adapts, no violations, cursor visible', () => { + const viewport = new Viewport(); + const input = { + editor: makeEditorRender(20, 10, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + // First render at 24 rows. Viewport scrollOffset is established. + const bigScreen = new MockScreen(80, 24); + const bigRenderer = new Renderer(bigScreen); + runPipeline(bigScreen, viewport, bigRenderer, input); + + // Resize to 10 rows. Viewport state (scrollOffset) persists across the resize. + const smallScreen = new MockScreen(80, 10); + const smallRenderer = new Renderer(smallScreen); + const { buffer, cursorRow, cursorCol } = layout(input); + const frame = viewport.resolve(buffer, 10, cursorRow, cursorCol); + smallRenderer.render(frame); + + smallScreen.assertNoScrollbackViolations(); + expect(frame.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(frame.visibleCursorRow).toBeLessThan(10); + }); + + it('writeHistory during active editor: no violations', () => { + const screen = new MockScreen(80, 20); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(3, 1, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + renderer.writeHistory('a history line'); + + screen.assertNoScrollbackViolations(); + }); + + it('question cleared before history write: zone re-renders without question', () => { + // Reproduces the bug where renderer.writeHistory re-rendered lastFrame + // (stale, containing the question) instead of the current layout state. + // Fix: writeHistoryLine + fresh runPipeline so zone reflects current state. + const screen = new MockScreen(80, 20); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + + // Initial render: zone includes a question component. + const withQuestion = { + editor: makeEditorRender(2, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: makeComponent(['Pick one:', '1) Yes', '2) No']), + columns: 80, + } satisfies LayoutInput; + runPipeline(screen, viewport, renderer, withQuestion); + + // Question is answered: clearQuestionLines() was called. Now history is + // written. The zone must re-render WITHOUT the question. + const withoutQuestion = { + editor: makeEditorRender(2, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + renderer.writeHistoryLine('answer: Yes'); + runPipeline(screen, viewport, renderer, withoutQuestion); + + screen.assertNoScrollbackViolations(); + // Question content must not appear in any visible row. + expect(screen.getRow(0)).toBe('answer: Yes'); + const visibleRows = Array.from({ length: 20 }, (_, i) => screen.getRow(i)); + expect(visibleRows.some((r) => r.includes('Pick one:'))).toBe(false); + }); + + it('cursor at first row of viewport: no violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(5, 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('first render with cursor mid-screen: no scrollback violations', () => { + // Simulates startup: previous output leaves cursor at row 7 of a 10-row screen. + // Content is 2 rows (status + 1 editor line). Viewport pads the frame to 10 rows. + // Without trimming, render() would write 10 rows from row 7, causing 7 scrollback + // violations (\n at row 9 fires 7 times). With trimming, only 2 rows are written + // (rows 7-8), leaving row 9 untouched. + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + screen.cursorRow = 7; + const input = { + editor: makeEditorRender(1, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('cursor at last row of viewport: no violations', () => { + const screen = new MockScreen(80, 5); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(5, 4, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('short history line does not leave stale zone content on same row', () => { + // Reproduces the bug where writeHistoryLine lacked clearLine. + // A zone row (80-char wide) at zone top, then a shorter history line + // written there, left old zone content at cols history_len..79 visible. + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const wideRow = 'A'.repeat(80); + const input = { + editor: makeEditorRender(1, 0, 0), + status: makeComponent([wideRow]), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + // Write a history line shorter than the 80-char zone row. + const shortHistory = 'short line'; + renderer.writeHistoryLine(shortHistory); + // Re-render zone so the history row is now in scrollback. + const { buffer, cursorRow, cursorCol } = layout(input); + const frame = viewport.resolve(buffer, screen.rows, cursorRow, cursorCol); + renderer.render(frame); + + // Row 0 must contain only the history text, no stale zone content. + expect(screen.getRow(0)).toBe(shortHistory); + }); +}); From a374486f587c0fbdefb19c2402ba3ced988e6164 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 16:23:46 +1100 Subject: [PATCH 10/15] Fix grapheme cluster splitting in line wrap --- .claude/sessions/2026-03-29.md | 8 ++++++++ src/Layout.ts | 10 ++++++---- test/layout.spec.ts | 22 ++++++++++++++++++++++ 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 989bf21..3803bdd 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -48,6 +48,14 @@ - Next: Manual verification by Supreme Commander, then commit/push/PR with `Closes #146`. - Violations: None +### 16:15 - fix/terminal-rendering-redesign (#146) Grapheme fix + +- Did: Fixed `wrapLine` to iterate grapheme clusters via `Intl.Segmenter` instead of code points. Added test `multi-codepoint grapheme clusters wrap as single units` to `test/layout.spec.ts` covering ZWJ emoji sequences. All 169 tests pass. +- Files: `src/Layout.ts`, `test/layout.spec.ts` +- Decisions: `for (const char of line)` iterates Unicode code points; multi-codepoint grapheme clusters (e.g. 👨‍👩‍👦 = U+1F468 ZWJ U+1F469 ZWJ U+1F466) would be split mid-cluster, causing incorrect wrap counts. Fix mirrors the pattern already used in `src/editor.ts`. +- Next: Await PR auto-merge. +- Violations: None + ### 15:46 - fix/terminal-rendering-redesign (#146) Commit/Push/PR - Did: Committed Stage 5 changes after supervisor manual verification. Pushed branch. Created PR with `Closes #146`. diff --git a/src/Layout.ts b/src/Layout.ts index 94b16f7..804c128 100644 --- a/src/Layout.ts +++ b/src/Layout.ts @@ -1,6 +1,8 @@ import stringWidth from 'string-width'; import type { EditorRender } from './renderer.js'; +const segmenter = new Intl.Segmenter(undefined, { granularity: 'grapheme' }); + /** * Output from an existing builder (status, attachment, preview). * `rows` are logical lines as the builder produces them (may be wider than columns). @@ -43,14 +45,14 @@ function wrapLine(line: string, columns: number): string[] { const segments: string[] = []; let current = ''; let currentWidth = 0; - for (const char of line) { - const cw = stringWidth(char); + for (const { segment } of segmenter.segment(line)) { + const cw = stringWidth(segment); if (currentWidth + cw > columns) { segments.push(current); - current = char; + current = segment; currentWidth = cw; } else { - current += char; + current += segment; currentWidth += cw; } } diff --git a/test/layout.spec.ts b/test/layout.spec.ts index 1045984..a8bf051 100644 --- a/test/layout.spec.ts +++ b/test/layout.spec.ts @@ -93,6 +93,28 @@ describe('layout', () => { expect(result.buffer[2]).toBe('A'.repeat(40)); }); + it('multi-codepoint grapheme clusters wrap as single units', () => { + // 👨‍👩‍👦 is one grapheme (U+1F468 ZWJ U+1F469 ZWJ U+1F466), display width 2. + // Iterating by code point splits the ZWJ sequence: the ZWJ (width 0) and each + // base emoji (width 2) are counted separately, causing the grapheme to be split + // across rows and producing more buffer rows than the actual display width warrants. + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F466}'; + const line = `AB${familyEmoji}C`; // display width: 1+1+2+1 = 5 + const input: LayoutInput = { + editor: makeEditor([line], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 4, + }; + const result = layout(input); + // "AB👨‍👩‍👦" fits in 4 columns (1+1+2=4), "C" wraps to next row. + expect(result.buffer).toHaveLength(2); + expect(result.buffer[0]).toBe(`AB${familyEmoji}`); + expect(result.buffer[1]).toBe('C'); + }); + it('50 editor lines: buffer has 50 rows, cursorRow accounts for non-editor rows', () => { const lines = Array.from({ length: 50 }, (_, i) => `line ${i}`); const input: LayoutInput = { From 94d1744166acca488627921aec89ad6569b14b51 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 16:51:09 +1100 Subject: [PATCH 11/15] Strip ZWJ characters before line wrap to match terminal rendering --- .claude/CLAUDE.md | 1 + .claude/sessions/2026-03-29.md | 8 +++++++ src/Layout.ts | 8 ++++--- src/sanitise.ts | 4 ++++ test/layout.spec.ts | 44 ++++++++++++++++++++++++++-------- test/sanitise.spec.ts | 42 +++++++++++++++++++++++++++++++- 6 files changed, 93 insertions(+), 14 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 078dc3f..e16db68 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -188,6 +188,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she - **Structured command execution via in-process MCP** (#99) — replaced freeform Bash with a structured Exec tool served by an in-process MCP server. Glob-based auto-approve (`execAutoApprove`) with custom zero-dep glob matcher (no minimatch dependency). - **Exec tool extracted to `@shellicar/mcp-exec`** — schema, executor, pipeline, validation rules, and ANSI stripping moved to a published package. CLI retains only `autoApprove.ts` (CLI-specific config concern). +- **ZWJ sanitisation in layout pipeline**: `sanitiseZwj` strips U+200D before `wrapLine` measures width. Terminals render ZWJ sequences as individual emojis; `string-width` assumes composed form. Stripping at the layout boundary removes the mismatch. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 3803bdd..e8c8a28 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -64,6 +64,14 @@ - Next: Await PR auto-merge. - Violations: None +### 16:50 - fix/terminal-rendering-redesign: ZWJ sanitisation in layout pipeline + +- Did: Added `sanitiseZwj` to `src/sanitise.ts` (strips U+200D before width measurement). Wired it into `wrapLine` in `src/Layout.ts` so terminal rendering and `string-width` agree on emoji width. Updated tests to reflect decomposed behavior. +- Files: `src/sanitise.ts`, `src/Layout.ts`, `test/sanitise.spec.ts`, `test/layout.spec.ts` +- Decisions: ZWJ sequences render as individual emojis in terminals that do not support composed rendering; `string-width` reports them as composed (width 2). Stripping ZWJ at the layout boundary removes the ambiguity. The old "wrap as single unit" test was testing the wrong assumption and was updated. Manual verification by SC still pending. +- Next: SC to verify manually in terminal. Then fix cursor position bug. +- Violations: None + ### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 - Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed. diff --git a/src/Layout.ts b/src/Layout.ts index 804c128..f286bdf 100644 --- a/src/Layout.ts +++ b/src/Layout.ts @@ -1,5 +1,6 @@ import stringWidth from 'string-width'; import type { EditorRender } from './renderer.js'; +import { sanitiseZwj } from './sanitise.js'; const segmenter = new Intl.Segmenter(undefined, { granularity: 'grapheme' }); @@ -39,13 +40,14 @@ export interface LayoutResult { * Returns at least one entry (empty string for empty input). */ function wrapLine(line: string, columns: number): string[] { - if (stringWidth(line) <= columns) { - return [line]; + const sanitised = sanitiseZwj(line); + if (stringWidth(sanitised) <= columns) { + return [sanitised]; } const segments: string[] = []; let current = ''; let currentWidth = 0; - for (const { segment } of segmenter.segment(line)) { + for (const { segment } of segmenter.segment(sanitised)) { const cw = stringWidth(segment); if (currentWidth + cw > columns) { segments.push(current); diff --git a/src/sanitise.ts b/src/sanitise.ts index 456fecc..b383604 100644 --- a/src/sanitise.ts +++ b/src/sanitise.ts @@ -3,3 +3,7 @@ const LONE_SURROGATE_RE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { expect(result.buffer[2]).toBe('A'.repeat(40)); }); - it('multi-codepoint grapheme clusters wrap as single units', () => { - // 👨‍👩‍👦 is one grapheme (U+1F468 ZWJ U+1F469 ZWJ U+1F466), display width 2. - // Iterating by code point splits the ZWJ sequence: the ZWJ (width 0) and each - // base emoji (width 2) are counted separately, causing the grapheme to be split - // across rows and producing more buffer rows than the actual display width warrants. + it('ZWJ sequence is decomposed: each emoji wraps independently', () => { + // ZWJ sequences are stripped before layout so string-width matches terminal rendering. + // 👨‍👩‍👦 (U+1F468 ZWJ U+1F469 ZWJ U+1F466) becomes 3 individual emojis, each width 2. + // Total display width: A(1)+B(1)+man(2)+woman(2)+girl(2)+C(1) = 9, not 5. const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F466}'; - const line = `AB${familyEmoji}C`; // display width: 1+1+2+1 = 5 + const line = `AB${familyEmoji}C`; const input: LayoutInput = { editor: makeEditor([line], 0, 0), status: null, @@ -108,11 +107,14 @@ describe('layout', () => { question: null, columns: 4, }; + const result = layout(input); - // "AB👨‍👩‍👦" fits in 4 columns (1+1+2=4), "C" wraps to next row. - expect(result.buffer).toHaveLength(2); - expect(result.buffer[0]).toBe(`AB${familyEmoji}`); - expect(result.buffer[1]).toBe('C'); + + // Row 0: A(1)+B(1)+man(2) = 4. Row 1: woman(2)+girl(2) = 4. Row 2: C(1). + expect(result.buffer).toHaveLength(3); + expect(result.buffer[0]).toBe('AB\u{1F468}'); + expect(result.buffer[1]).toBe('\u{1F469}\u{1F466}'); + expect(result.buffer[2]).toBe('C'); }); it('50 editor lines: buffer has 50 rows, cursorRow accounts for non-editor rows', () => { @@ -189,6 +191,28 @@ describe('layout', () => { expect(result.cursorRow).toBe(3); }); + it('ZWJ family emoji at col 79: wraps to next line (terminal renders 4 emojis x 2 = 8 cells)', () => { + // string-width reports the ZWJ sequence as width 2 (composed form). + // The terminal renders it as 4 separate emojis, each width 2 = 8 cells total. + // 78 ASCII + 8 emoji cells = 86 > 80, so the emoji must wrap. + // Without ZWJ stripping, layout sees 78+2=80 and returns 1 buffer row (wrong). + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const line = 'A'.repeat(78) + familyEmoji; + const input: LayoutInput = { + editor: makeEditor([line], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + + const result = layout(input); + + // 78 A's + first emoji (2 cols) = 80 on row 0; remaining 3 emojis on row 1. + expect(result.buffer.length).toBe(2); + }); + it('editor cursor on wrapped line: cursorRow offset by editorStartRow', () => { // Editor has 1 wrapped line (160 chars = 2 visual rows at 80 cols) // cursor is on visual row 1 of the editor (second wrap segment) diff --git a/test/sanitise.spec.ts b/test/sanitise.spec.ts index 456ecf0..ec89a54 100644 --- a/test/sanitise.spec.ts +++ b/test/sanitise.spec.ts @@ -1,5 +1,6 @@ +import stringWidth from 'string-width'; import { describe, expect, it } from 'vitest'; -import { sanitiseLoneSurrogates } from '../src/sanitise.js'; +import { sanitiseLoneSurrogates, sanitiseZwj } from '../src/sanitise.js'; describe('sanitiseLoneSurrogates', () => { it('replaces lone high surrogate', () => { @@ -26,3 +27,42 @@ describe('sanitiseLoneSurrogates', () => { expect(actual).toBe(expected); }); }); + +// Bug: string-width treats ZWJ sequences as composed (width 2), +// but terminals render them as individual emojis (4 x 2 = 8 cells). +// This mismatch causes wrong line height and cursor position. +describe('sanitiseZwj', () => { + it('raw ZWJ sequence: stringWidth returns 2 not 8, exposing the terminal mismatch', () => { + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + + const actual = stringWidth(familyEmoji); + + expect(actual).toBe(2); + }); + + it('strips ZWJ characters from a compound emoji sequence', () => { + const input = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const expected = '\u{1F468}\u{1F469}\u{1F467}\u{1F466}'; + + const actual = sanitiseZwj(input); + + expect(actual).toBe(expected); + }); + + it('stringWidth of sanitised sequence equals sum of individual emoji widths', () => { + const input = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const expected = 8; // 4 emojis x 2 cells each + + const actual = stringWidth(sanitiseZwj(input)); + + expect(actual).toBe(expected); + }); + + it('leaves text without ZWJ unchanged', () => { + const input = 'hello world'; + + const actual = sanitiseZwj(input); + + expect(actual).toBe(input); + }); +}); From ff82c8131d2993aeb3b6a4d1f9d9ec7d63c59195 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 17:37:34 +1100 Subject: [PATCH 12/15] Fix cursor position when content fills terminal width --- src/TerminalRenderer.ts | 8 +-- src/renderer.ts | 3 +- test/MockScreen.ts | 15 ++++- ...derer.spec.ts => TerminalRenderer.spec.ts} | 62 +++++++++++++++++++ test/mock-screen.spec.ts | 16 +++-- test/prepareEditor.spec.ts | 44 +++++++++++++ 6 files changed, 132 insertions(+), 16 deletions(-) rename test/{renderer.spec.ts => TerminalRenderer.spec.ts} (70%) create mode 100644 test/prepareEditor.spec.ts diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts index 31aaff1..b50faca 100644 --- a/src/TerminalRenderer.ts +++ b/src/TerminalRenderer.ts @@ -1,4 +1,3 @@ -import stringWidth from 'string-width'; import type { Screen } from './Screen.js'; import type { ViewportResult } from './Viewport.js'; @@ -40,12 +39,7 @@ export class Renderer { out += this.buildZoneOutput(renderFrame); out += showCursor + syncEnd; this.zoneHeight = renderFrame.rows.length; - // Track the actual visual cursor row offset from zone top. Rows that are - // exactly `columns` wide wrap the cursor to the next row, so the visual - // offset is greater than the logical row index when any row above the - // cursor wraps. - const wrapAboveCursor = renderFrame.rows.slice(0, renderFrame.visibleCursorRow).reduce((sum, row) => sum + Math.floor(stringWidth(row) / this.screen.columns), 0); - this.lastVisibleCursorRow = renderFrame.visibleCursorRow + wrapAboveCursor; + this.lastVisibleCursorRow = renderFrame.visibleCursorRow; this.lastFrame = frame; this.screen.write(out); } diff --git a/src/renderer.ts b/src/renderer.ts index 860dba1..1682de4 100644 --- a/src/renderer.ts +++ b/src/renderer.ts @@ -25,13 +25,14 @@ export function prepareEditor(editor: EditorState, prompt: string): EditorRender const cursorPrefix = editor.cursor.row === 0 ? prompt : CONTINUATION; const textBeforeCursor = editor.lines[editor.cursor.row].slice(0, editor.cursor.col); - const cursorCol = stringWidth(cursorPrefix) + stringWidth(textBeforeCursor); + let cursorCol = stringWidth(cursorPrefix) + stringWidth(textBeforeCursor); let cursorRow = 0; for (let i = 0; i < editor.cursor.row; i++) { cursorRow += Math.max(1, Math.ceil(stringWidth(lines[i]) / columns)); } cursorRow += Math.floor(cursorCol / columns); + cursorCol = cursorCol % columns; return { lines, cursorRow, cursorCol }; } diff --git a/test/MockScreen.ts b/test/MockScreen.ts index 982533e..b4576f9 100644 --- a/test/MockScreen.ts +++ b/test/MockScreen.ts @@ -5,6 +5,7 @@ export class MockScreen implements Screen { public cursorRow = 0; public cursorCol = 0; public scrollbackViolations = 0; + private pendingWrap = false; public constructor( public readonly columns: number, @@ -35,9 +36,11 @@ export class MockScreen implements Screen { } } else if (ch === '\r') { this.cursorCol = 0; + this.pendingWrap = false; i++; } else if (ch === '\n') { this.cursorCol = 0; + this.pendingWrap = false; if (this.cursorRow === this.rows - 1) { this.scrollbackViolations++; this.cells.shift(); @@ -47,9 +50,8 @@ export class MockScreen implements Screen { } i++; } else { - this.cells[this.cursorRow][this.cursorCol] = ch; - this.cursorCol++; - if (this.cursorCol >= this.columns) { + if (this.pendingWrap) { + this.pendingWrap = false; this.cursorCol = 0; if (this.cursorRow < this.rows - 1) { this.cursorRow++; @@ -59,12 +61,19 @@ export class MockScreen implements Screen { this.cells.push(new Array(this.columns).fill('')); } } + this.cells[this.cursorRow][this.cursorCol] = ch; + this.cursorCol++; + if (this.cursorCol >= this.columns) { + this.cursorCol = this.columns - 1; + this.pendingWrap = true; + } i++; } } } private handleCsi(param: string, final: string): void { + this.pendingWrap = false; switch (final) { case 'A': { const n = parseInt(param || '1', 10); diff --git a/test/renderer.spec.ts b/test/TerminalRenderer.spec.ts similarity index 70% rename from test/renderer.spec.ts rename to test/TerminalRenderer.spec.ts index 80c694b..1dcb528 100644 --- a/test/renderer.spec.ts +++ b/test/TerminalRenderer.spec.ts @@ -1,13 +1,75 @@ import { describe, expect, it } from 'vitest'; +import type { Screen } from '../src/Screen.js'; import { Renderer } from '../src/TerminalRenderer.js'; import type { ViewportResult } from '../src/Viewport.js'; import { MockScreen } from './MockScreen.js'; +function makeScreen(columns: number) { + const output: string[] = []; + const screen: Screen = { + columns, + rows: 24, + write(data: string) { + output.push(data); + }, + onResize() { + return () => {}; + }, + }; + return { screen, output }; +} + function makeFrame(rows: string[], visibleCursorRow: number, visibleCursorCol: number): ViewportResult { return { rows, visibleCursorRow, visibleCursorCol }; } describe('Renderer', () => { + describe('lastVisibleCursorRow after render', () => { + it('does not overcount when a row above the cursor exactly fills terminal width', () => { + const cols = 10; + const { screen, output } = makeScreen(cols); + const renderer = new Renderer(screen); + + // Row 0 is exactly cols-wide; cursor sits on row 1 + const frame = { + rows: ['a'.repeat(cols), 'x'], + visibleCursorRow: 1, + visibleCursorCol: 1, + }; + + renderer.render(frame); + output.length = 0; + + // Second render: should move up by 1 (lastVisibleCursorRow = 1), not 2 + renderer.render(frame); + const all = output.join(''); + + expect(all).toContain('\x1B[1A'); // cursorUp(1): correct + expect(all).not.toContain('\x1B[2A'); // cursorUp(2): was the bug + }); + + it('moves up correctly when no row above the cursor fills terminal width', () => { + const cols = 10; + const { screen, output } = makeScreen(cols); + const renderer = new Renderer(screen); + + const frame = { + rows: ['abc', 'cursor'], + visibleCursorRow: 1, + visibleCursorCol: 6, + }; + + renderer.render(frame); + output.length = 0; + + renderer.render(frame); + const all = output.join(''); + + expect(all).toContain('\x1B[1A'); + expect(all).not.toContain('\x1B[2A'); + }); + }); + it('single frame on 10-row screen: no scrollback violations', () => { const screen = new MockScreen(80, 10); const renderer = new Renderer(screen); diff --git a/test/mock-screen.spec.ts b/test/mock-screen.spec.ts index 6dd4e7a..bdbcb9a 100644 --- a/test/mock-screen.spec.ts +++ b/test/mock-screen.spec.ts @@ -121,18 +121,24 @@ describe('MockScreen', () => { expect(() => screen.assertNoScrollbackViolations()).toThrow(); }); - it('writing fills a row and wraps to the next row', () => { + it('writing fills a row enters pending-wrap; next char wraps to next row', () => { const screen = new MockScreen(5, 24); - screen.write('ABCDE'); // 5 chars fills 5-col row, wraps - expect(screen.cursorRow).toBe(1); - expect(screen.cursorCol).toBe(0); + screen.write('ABCDE'); // 5 chars fills 5-col row, enters pending-wrap at last col + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(4); expect(screen.getRow(0)).toBe('ABCDE'); + screen.write('F'); // next char triggers wrap to row 1 + expect(screen.cursorRow).toBe(1); + expect(screen.cursorCol).toBe(1); + expect(screen.getRow(1)).toBe('F'); }); it('writing past end of last row causes scrollback violation', () => { const screen = new MockScreen(5, 2); screen.write('\n'); // row 1 (last row for 2-row screen) - screen.write('ABCDE'); // fills last row, wrap -> scrollback violation + screen.write('ABCDE'); // fills last row, enters pending-wrap, no violation yet + expect(screen.scrollbackViolations).toBe(0); + screen.write('F'); // triggers wrap past last row, scrollback violation expect(screen.scrollbackViolations).toBe(1); }); diff --git a/test/prepareEditor.spec.ts b/test/prepareEditor.spec.ts new file mode 100644 index 0000000..06072ef --- /dev/null +++ b/test/prepareEditor.spec.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import type { EditorState } from '../src/editor.js'; +import { prepareEditor } from '../src/renderer.js'; + +// In test (non-TTY) environments process.stdout.columns is undefined, +// so renderer falls back to 80 columns via `|| 80`. +const COLUMNS = 80; +const PROMPT = '> '; // visual width 2 + +function editor(text: string, col?: number): EditorState { + return { lines: [text], cursor: { row: 0, col: col ?? text.length } }; +} + +describe('prepareEditor', () => { + describe('cursorCol is always within-row (< columns), not the absolute offset', () => { + it('cursor at exact wrap boundary wraps cursorCol to 0', () => { + // prompt(2) + text(78) = 80 = COLUMNS: cursor is at the start of visual row 1 + const result = prepareEditor(editor('a'.repeat(78)), PROMPT); + expect(result.cursorRow).toBe(1); + expect(result.cursorCol).toBe(0); + }); + + it('cursor past wrap boundary gives correct within-row offset', () => { + // prompt(2) + text(83) = 85: visual row 1, column 5 + const result = prepareEditor(editor('a'.repeat(83)), PROMPT); + expect(result.cursorRow).toBe(1); + expect(result.cursorCol).toBe(5); + }); + + it('cursor before any wrap is unchanged', () => { + // prompt(2) + text(10) = 12, stays on row 0 + const result = prepareEditor(editor('a'.repeat(10)), PROMPT); + expect(result.cursorRow).toBe(0); + expect(result.cursorCol).toBe(12); + }); + + it('cursorCol is always less than terminal width for all wrap positions', () => { + for (const len of [77, 78, 79, 80, 81, 158, 159, 160, 161]) { + const result = prepareEditor(editor('a'.repeat(len)), PROMPT); + expect(result.cursorCol, `len=${len}`).toBeLessThan(COLUMNS); + } + }); + }); +}); From f7d4db7bd70dca18bd1289a0d10ad6e756138081 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 18:33:35 +1100 Subject: [PATCH 13/15] Fix zone corruption after terminal resize --- .claude/CLAUDE.md | 2 +- .claude/sessions/2026-03-29.md | 8 ++++++++ src/ClaudeCli.ts | 1 + src/TerminalRenderer.ts | 27 ++++++++++++++++++++++++++- src/terminal.ts | 4 ++++ test/MockScreen.ts | 9 +++++++++ test/TerminalRenderer.spec.ts | 25 +++++++++++++++++++++++++ test/mock-screen.spec.ts | 22 ++++++++++++++++++++++ 8 files changed, 96 insertions(+), 2 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index e16db68..9cfe803 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: All stages complete. PR created, awaiting merge. +In-progress: All stages complete, PR created. Post-PR resize corruption fix applied; awaiting SC manual verification and merge. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index e8c8a28..91cd058 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -72,6 +72,14 @@ - Next: SC to verify manually in terminal. Then fix cursor position bug. - Violations: None +### 18:30 - fix/terminal-rendering-redesign: resize corruption fix + +- Did: Diagnosed resize corruption: on terminal narrow, old zone content reflows to more rows at the new column width and can appear anywhere in the visible area. `lastVisibleCursorRow` is stale and `cursorUp` moves to the wrong row. Added TDD test to expose the bug (RED: scrollback violation). Implemented fix: `Renderer.notifyResize()` sets `pendingResize` flag; next `render()` clears the entire visible screen (`cursorAt(1,1)` + `clearDown`), then anchors the new zone at `screen.rows - zoneHeight`. Added `MockScreen` support for `ESC[row;colH` absolute positioning (required by fix). Wired `Terminal.notifyResize()` and `ClaudeCli` resize debounce. 184 tests pass. SC manual verification: fix partially resolved the issue; second iteration replaced partial clear (old zone area only) with full screen clear, which also handles the case where reflowed zone content spreads above the calculated old zone top. Build and type-check clean. +- Files: `src/TerminalRenderer.ts`, `src/terminal.ts`, `src/ClaudeCli.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts`, `test/TerminalRenderer.spec.ts` +- Decisions: Full screen clear on resize is the correct approach because after terminal reflow the old zone can occupy an unknown number of rows anywhere in the visible area. Partial clear (based on stored zoneHeight) is unreliable. Trade-off: visible history above zone is momentarily blank after resize (still accessible in scrollback). +- Next: SC to verify manually with latest build. +- Violations: None + ### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 - Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed. diff --git a/src/ClaudeCli.ts b/src/ClaudeCli.ts index 776545c..6a6f0a7 100644 --- a/src/ClaudeCli.ts +++ b/src/ClaudeCli.ts @@ -992,6 +992,7 @@ export class ClaudeCli { clearTimeout(this.resizeTimer); this.resizeTimer = setTimeout(() => { this.term.paused = false; + this.term.notifyResize(); this.redraw(); }, 300); }); diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts index b50faca..19ffbf9 100644 --- a/src/TerminalRenderer.ts +++ b/src/TerminalRenderer.ts @@ -4,6 +4,7 @@ import type { ViewportResult } from './Viewport.js'; const ESC = '\x1B['; const cursorUp = (n: number) => (n > 0 ? `${ESC}${n}A` : ''); const cursorTo = (col: number) => `${ESC}${col + 1}G`; +const cursorAt = (row: number, col: number) => `${ESC}${row};${col}H`; // 1-based const clearLine = `${ESC}2K`; const clearDown = `${ESC}J`; const showCursor = `${ESC}?25h`; @@ -15,9 +16,20 @@ export class Renderer { public zoneHeight = 0; private lastVisibleCursorRow = 0; private lastFrame: ViewportResult | null = null; + private pendingResize = false; public constructor(private readonly screen: Screen) {} + /** + * Call after a terminal resize. The physical cursor position is unknown after + * terminal content reflows, so lastVisibleCursorRow is stale. The next render() + * call will use absolute positioning to re-anchor the zone at the screen bottom + * instead of the stale relative cursorUp. + */ + public notifyResize(): void { + this.pendingResize = true; + } + public render(frame: ViewportResult): void { // Trim trailing empty rows from the Viewport-padded frame. Padding is correct // for Viewport's contract but would cause the Renderer to write screenRows rows @@ -35,7 +47,20 @@ export class Renderer { visibleCursorCol: frame.visibleCursorCol, }; let out = syncStart + hideCursor; - out += cursorUp(this.lastVisibleCursorRow); + if (this.pendingResize) { + this.pendingResize = false; + // After a resize, the terminal reflows all content. The old zone may now + // occupy a different number of rows at the new column width and can end up + // anywhere in the visible area. Clear the entire visible screen so no + // reflowed stale zone rows remain, then anchor the new zone at the bottom. + const newZoneHeight = renderFrame.rows.length; + const newZoneTop = this.screen.rows - newZoneHeight + 1; // 1-based + out += cursorAt(1, 1); + out += clearDown; + out += cursorAt(newZoneTop, 1); + } else { + out += cursorUp(this.lastVisibleCursorRow); + } out += this.buildZoneOutput(renderFrame); out += showCursor + syncEnd; this.zoneHeight = renderFrame.rows.length; diff --git a/src/terminal.ts b/src/terminal.ts index 2158e9b..d81b24b 100644 --- a/src/terminal.ts +++ b/src/terminal.ts @@ -50,6 +50,10 @@ export class Terminal { this.drowningThreshold = drowningThreshold; } + public notifyResize(): void { + this.renderer.notifyResize(); + } + public get paused(): boolean { return this._paused; } diff --git a/test/MockScreen.ts b/test/MockScreen.ts index b4576f9..0b81818 100644 --- a/test/MockScreen.ts +++ b/test/MockScreen.ts @@ -93,6 +93,15 @@ export class MockScreen implements Screen { } break; } + case 'H': { + // Cursor position: ESC[row;colH (1-based, defaults to 1;1) + const parts = param.split(';'); + const row = parseInt(parts[0] || '1', 10); + const col = parseInt(parts[1] || '1', 10); + this.cursorRow = Math.max(0, Math.min(this.rows - 1, row - 1)); + this.cursorCol = Math.max(0, Math.min(this.columns - 1, col - 1)); + break; + } case 'J': { for (let r = this.cursorRow; r < this.rows; r++) { for (let c = 0; c < this.columns; c++) { diff --git a/test/TerminalRenderer.spec.ts b/test/TerminalRenderer.spec.ts index 1dcb528..ab1e202 100644 --- a/test/TerminalRenderer.spec.ts +++ b/test/TerminalRenderer.spec.ts @@ -157,4 +157,29 @@ describe('Renderer', () => { expect(screen.cursorRow).toBe(3); expect(screen.cursorCol).toBe(15); }); + + describe('resize: simulated terminal reflow', () => { + it('no scrollback violations when cursor displaced by terminal reflow before render', () => { + // Resize bug: when the terminal is resized, the physical cursor moves because + // content reflows. renderer.lastVisibleCursorRow is stale (based on the + // pre-resize frame). The next render does cursorUp(stale), which moves the + // cursor from the wrong row. A taller zone then overflows the screen bottom. + + const screen = new MockScreen(80, 5); + const renderer = new Renderer(screen); + + // Phase 1: render a 2-row zone; cursor lands on row 1 + renderer.render(makeFrame(['first0', 'first1'], 1, 0)); + expect(screen.cursorRow).toBe(1); + + // Simulate terminal resize+reflow: cursor has been displaced to row 4 + screen.cursorRow = 4; + renderer.notifyResize(); + + // Phase 2: render a 4-row zone (zone grew because the terminal is now narrower) + renderer.render(makeFrame(['new0', 'new1', 'new2', 'new3'], 3, 0)); + + screen.assertNoScrollbackViolations(); + }); + }); }); diff --git a/test/mock-screen.spec.ts b/test/mock-screen.spec.ts index bdbcb9a..64e8a13 100644 --- a/test/mock-screen.spec.ts +++ b/test/mock-screen.spec.ts @@ -169,4 +169,26 @@ describe('MockScreen', () => { const screen = new MockScreen(80, 24); expect(screen.getRow(0)).toBe(''); }); + + it('ESC[row;colH moves cursor to absolute position (1-based)', () => { + const screen = new MockScreen(80, 24); + screen.write('\x1B[5;10H'); // row 5, col 10 (1-based) + expect(screen.cursorRow).toBe(4); // 0-based + expect(screen.cursorCol).toBe(9); // 0-based + }); + + it('ESC[1;1H moves cursor to top-left', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n\n'); // row 3 + screen.write('\x1B[1;1H'); + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(0); + }); + + it('ESC[row;colH clamps to screen boundaries', () => { + const screen = new MockScreen(10, 5); + screen.write('\x1B[99;99H'); + expect(screen.cursorRow).toBe(4); // clamped to rows - 1 + expect(screen.cursorCol).toBe(9); // clamped to columns - 1 + }); }); From 6686de5d71a5228df1e8d38b907db08c4bd71f37 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 19:41:42 +1100 Subject: [PATCH 14/15] Keep zone in place when terminal widens --- src/TerminalRenderer.ts | 22 +++++++----- test/TerminalRenderer.spec.ts | 67 +++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 8 deletions(-) diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts index 19ffbf9..7caa4d2 100644 --- a/src/TerminalRenderer.ts +++ b/src/TerminalRenderer.ts @@ -49,15 +49,21 @@ export class Renderer { let out = syncStart + hideCursor; if (this.pendingResize) { this.pendingResize = false; - // After a resize, the terminal reflows all content. The old zone may now - // occupy a different number of rows at the new column width and can end up - // anywhere in the visible area. Clear the entire visible screen so no - // reflowed stale zone rows remain, then anchor the new zone at the bottom. + // After a resize, the terminal reflows content. Two cases: + // - Narrowing (zone grew): the terminal may displace the cursor significantly + // as content wraps onto more rows. Use absolute positioning anchored at the + // screen bottom so the render starts from a known location. + // - Widening (zone shrunk or same): the cursor stays approximately in place + // because content unwraps and the cursor doesn't travel far. Treat like a + // normal render (cursorUp) so the zone stays where it was instead of + // jumping to the screen bottom. const newZoneHeight = renderFrame.rows.length; - const newZoneTop = this.screen.rows - newZoneHeight + 1; // 1-based - out += cursorAt(1, 1); - out += clearDown; - out += cursorAt(newZoneTop, 1); + if (newZoneHeight > this.zoneHeight) { + const newZoneTop = this.screen.rows - newZoneHeight + 1; // 1-based + out += cursorAt(newZoneTop, 1); + } else { + out += cursorUp(this.lastVisibleCursorRow); + } } else { out += cursorUp(this.lastVisibleCursorRow); } diff --git a/test/TerminalRenderer.spec.ts b/test/TerminalRenderer.spec.ts index ab1e202..f180c36 100644 --- a/test/TerminalRenderer.spec.ts +++ b/test/TerminalRenderer.spec.ts @@ -159,6 +159,73 @@ describe('Renderer', () => { }); describe('resize: simulated terminal reflow', () => { + it('widen resize: does not clear rows above new zone top', () => { + // Bug: pendingResize path does cursorAt(1,1) + clearDown, erasing the entire + // visible screen. When widening, the zone shrinks and more rows above become + // visible (OS reflow). Those rows must NOT be cleared by the resize render. + + // 10-row screen. Initial zone: 5 rows at rows 5–9 (0-indexed). + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + + renderer.render(makeFrame(['zone0', 'zone1', 'zone2', 'zone3', 'zone4'], 0, 0)); + + // Simulate OS widen reflow: old 5-row zone reflowed to 3 rows at new width. + // Terminal now shows history at rows 0–6, old zone at rows 7–9. + // Write history content directly into cells to represent what is visible. + for (let r = 0; r < 7; r++) { + const text = `history ${r}`; + for (let c = 0; c < text.length; c++) { + screen.cells[r][c] = text[c] as string; + } + } + screen.cursorRow = 7; + screen.cursorCol = 0; + + renderer.notifyResize(); + + // New zone: 3 rows (wider terminal, less wrapping). newZoneTop = 10 - 3 + 1 = 8 (1-based) = row 7 (0-based). + renderer.render(makeFrame(['zone0', 'zone1', 'zone2'], 0, 0)); + + // History rows 0–6 must be preserved (not cleared by pendingResize). + expect(screen.getRow(0)).toBe('history 0'); + expect(screen.getRow(6)).toBe('history 6'); + + // New zone must be at rows 7–9. + expect(screen.getRow(7)).toBe('zone0'); + expect(screen.getRow(8)).toBe('zone1'); + expect(screen.getRow(9)).toBe('zone2'); + }); + + it('widen resize: zone stays in place instead of jumping to screen bottom', () => { + // Bug: pendingResize used cursorAt(newZoneTop) which always anchors the zone + // to the bottom of the screen. When widening, the cursor doesn't move, so the + // zone should stay where it was (rows 0–2), not jump to the bottom (rows 7–9). + + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + + // Initial render: 5-row zone at rows 0–4 (cursor at visibleCursorRow=0) + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 0, 0)); + expect(screen.cursorRow).toBe(0); + + renderer.notifyResize(); + + // Widen: new 3-row zone (less wrapping at wider columns) + renderer.render(makeFrame(['z0', 'z1', 'z2'], 0, 0)); + + // Zone must stay at rows 0–2, not jump to rows 7–9 + const actual0 = screen.getRow(0); + const actual2 = screen.getRow(2); + const actual7 = screen.getRow(7); + + expect(actual0).toBe('z0'); + expect(actual2).toBe('z2'); + expect(actual7).toBe(''); + + screen.assertNoScrollbackViolations(); + }); + it('no scrollback violations when cursor displaced by terminal reflow before render', () => { // Resize bug: when the terminal is resized, the physical cursor moves because // content reflows. renderer.lastVisibleCursorRow is stale (based on the From dc204dae65c90cf173dd488916fdb3a5902b3bc0 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Sun, 29 Mar 2026 19:43:21 +1100 Subject: [PATCH 15/15] Session log: widen resize zone-anchoring fix --- .claude/CLAUDE.md | 2 +- .claude/sessions/2026-03-29.md | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9cfe803..b687577 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -73,7 +73,7 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State Branch: `fix/terminal-rendering-redesign` -In-progress: All stages complete, PR created. Post-PR resize corruption fix applied; awaiting SC manual verification and merge. +In-progress: All stages complete, PR #147 open. Resize corruption fix (narrow) and widen zone-anchoring fix both applied and pushed. Awaiting SC manual verification and merge. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index 91cd058..3190be6 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -80,6 +80,14 @@ - Next: SC to verify manually with latest build. - Violations: None +### 19:41 - fix/terminal-rendering-redesign: widen resize zone-anchoring fix + +- Did: Diagnosed two widen-resize bugs via TDD. Bug 1: `cursorAt(1,1) + clearDown` in the `pendingResize` path erased all visible history above the zone when widening. Bug 2: `cursorAt(newZoneTop, 1)` always anchored the zone to the screen bottom, causing it to jump down even when the cursor had not moved. Fixed both by differentiating narrow vs widen in `pendingResize`: use `cursorAt(newZoneTop, 1)` only when `newZoneHeight > zoneHeight` (zone grew, cursor displaced by reflow); use `cursorUp(lastVisibleCursorRow)` otherwise (zone shrunk or same, cursor approximately in place). Added two regression tests; all 186 tests pass. +- Files: `src/TerminalRenderer.ts`, `test/TerminalRenderer.spec.ts` +- Decisions: Zone-grew heuristic (`newZoneHeight > zoneHeight`) is the narrowing signal: when the terminal narrows, wrapped content produces more rows; when it widens, fewer rows. The existing narrow-case test (`no scrollback violations when cursor displaced`) continues to pass because `newZoneHeight (4) > zoneHeight (2)` triggers the absolute path correctly. +- Next: SC to verify manually with latest build. Await PR #147 auto-merge. +- Violations: None + ### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 - Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed.