smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tui): honest editor-height floor and dead-branch cleanup

Browse Source 
Addresses Copilot + Codex review on #2521:

- computeEditorMaxHeight returned 1 on terminals too small to host both the
  editor and the chrome reserve, but the bordered editor never renders fewer
  than 3 rows (2 border + 1 content). The cap now floors at that real minimum
  (EDITOR_MIN_RENDERED_ROWS) so it no longer misreports the rows the editor
  occupies; rendering is unchanged, and the contract is documented honestly
  (reserve holds once terminalRows >= 7).
- #resolveOverlayLayout now always resolves maxHeight (?? availHeight), so the
  maxHeight !== undefined branch in effectiveHeight, the composite slice guard,
  and the number | undefined return type were dead. Tightened all three.
- Mirrored the maxHeight-default contract in the render stress oracle
  (resolveExpectedOverlayLayout + compositeExpectedOverlays) so the randomized
  sweep validates the real clipped geometry instead of the obsolete unclipped
  one; updated the oracle helper test expectation accordingly.

Editor-height tests rewritten to assert the real contract (reserve when the
terminal can host both; pinned to the bordered minimum below that).
This commit is contained in:
metaphorics
2026-06-14 08:40:12 +09:00
parent b28ad9a1dc
commit a95be2860f
6 changed files with 42 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1 -1
View File
@@ -4,7 +4,7 @@
### Fixed
- Fixed the editor input box claiming a disproportionate share of small terminals (<=18 rows): the editor max-height floor (6 rows) ignored the available space. The height now yields to terminal size (`EDITOR_MIN_CHROME_ROWS`), always reserving rows for the transcript and status line.
- Fixed the editor input box claiming a disproportionate share of small terminals (<=18 rows): the editor max-height floor (6 rows) ignored the available space. The height now yields to terminal size (`EDITOR_MIN_CHROME_ROWS`), reserving rows for the transcript and status line whenever the terminal can host both; on terminals too small for both, the editor collapses to its real bordered minimum instead of overshooting a fictitious cap.
## [15.12.5] - 2026-06-13
### Changed
packages/coding-agent/src/modes/interactive-mode.ts
+15 -4
View File
@@ -210,13 +210,24 @@ const EDITOR_MAX_HEIGHT_MIN = 6;
const EDITOR_MAX_HEIGHT_MAX = 18;
const EDITOR_RESERVED_ROWS = 12;
const EDITOR_FALLBACK_ROWS = 24;
const EDITOR_MIN_CHROME_ROWS = 4; // rows reserved for transcript + status + chrome on tiny terms
const EDITOR_MIN_CHROME_ROWS = 4; // rows reserved for transcript + status on small terms
const EDITOR_MIN_RENDERED_ROWS = 3; // bordered editor floor: top+bottom border + 1 content row
/**
* Editor max-height cap for a terminal of `terminalRows` rows.
*
* Roomy terminals get the comfortable [6, 18] band. Small terminals shrink the
* cap so the editor leaves at least EDITOR_MIN_CHROME_ROWS rows for the
* transcript + status line. The editor is bordered, so it never renders fewer
* than EDITOR_MIN_RENDERED_ROWS rows; once the terminal is too small for both
* (terminalRows < EDITOR_MIN_RENDERED_ROWS + EDITOR_MIN_CHROME_ROWS) the cap is
* pinned to that floor — returning a smaller number would not shrink the editor
* any further, it would only misreport the rows it actually occupies.
*/
export function computeEditorMaxHeight(terminalRows: number): number {
const rows = Number.isFinite(terminalRows) && terminalRows > 0 ? terminalRows : EDITOR_FALLBACK_ROWS;
const desired = Math.max(EDITOR_MAX_HEIGHT_MIN, Math.min(EDITOR_MAX_HEIGHT_MAX, rows - EDITOR_RESERVED_ROWS));
// Never let the editor crowd out the rest of the UI on small terminals.
return Math.max(1, Math.min(desired, rows - EDITOR_MIN_CHROME_ROWS));
const comfortable = Math.max(EDITOR_MAX_HEIGHT_MIN, Math.min(EDITOR_MAX_HEIGHT_MAX, rows - EDITOR_RESERVED_ROWS));
return Math.max(EDITOR_MIN_RENDERED_ROWS, Math.min(comfortable, rows - EDITOR_MIN_CHROME_ROWS));
}
const HUD_NOTE_SUP_DIGITS: Record<string, string> = {
packages/coding-agent/test/editor-max-height.test.ts
+15 -3
View File
@@ -2,16 +2,28 @@ import { describe, expect, it } from "bun:test";
import { computeEditorMaxHeight } from "@oh-my-pi/pi-coding-agent/modes/interactive-mode";
describe("computeEditorMaxHeight", () => {
it("caps the editor while preserving chrome rows on small terminals", () => {
it("caps the editor within the comfortable band on roomy terminals", () => {
expect(computeEditorMaxHeight(30)).toBe(18);
expect(computeEditorMaxHeight(18)).toBe(6);
expect(computeEditorMaxHeight(8)).toBe(4);
expect(computeEditorMaxHeight(5)).toBe(1);
expect(computeEditorMaxHeight(Number.NaN)).toBe(12);
expect(computeEditorMaxHeight(0)).toBe(12);
});
for (let rows = 5; rows <= 18; rows += 1) {
it("reserves at least four chrome rows once the terminal can host both", () => {
// Editor floor (3 rendered rows, bordered) + chrome reserve (4) = 7 rows.
for (let rows = 7; rows <= 18; rows += 1) {
expect(rows - computeEditorMaxHeight(rows)).toBeGreaterThanOrEqual(4);
}
});
it("pins the cap to the bordered editor's real minimum on tinier terminals", () => {
// Below 7 rows there is no room for both; the cap collapses to the editor's
// real rendered floor (2 border + 1 content) rather than a fictitious value
// the editor would silently overshoot.
expect(computeEditorMaxHeight(6)).toBe(3);
expect(computeEditorMaxHeight(5)).toBe(3);
expect(computeEditorMaxHeight(4)).toBe(3);
expect(computeEditorMaxHeight(1)).toBe(3);
});
});
packages/tui/src/tui.ts
+5 -4
View File
@@ -1875,7 +1875,7 @@ export class TUI extends Container {
overlayHeight: number,
termWidth: number,
termHeight: number,
): { width: number; row: number; col: number; maxHeight: number | undefined } {
): { width: number; row: number; col: number; maxHeight: number } {
const opt = options ?? {};
// Parse margin (clamp to non-negative)
@@ -1905,8 +1905,9 @@ export class TUI extends Container {
let maxHeight = parseSizeValue(opt.maxHeight, termHeight) ?? availHeight;
maxHeight = Math.max(1, Math.min(maxHeight, availHeight));
// Effective overlay height (may be clamped by maxHeight)
const effectiveHeight = maxHeight !== undefined ? Math.min(overlayHeight, maxHeight) : overlayHeight;
// Effective overlay height: maxHeight is always resolved (defaults to
// availHeight above), so the overlay is unconditionally clamped to fit.
const effectiveHeight = Math.min(overlayHeight, maxHeight);
// === Resolve position ===
let row: number;
@@ -2017,7 +2018,7 @@ export class TUI extends Container {
// (width and maxHeight don't depend on overlay height).
const { width, maxHeight } = this.#resolveOverlayLayout(options, 0, termWidth, termHeight);
let overlayLines = component.render(width);
if (maxHeight !== undefined && overlayLines.length > maxHeight) {
if (overlayLines.length > maxHeight) {
overlayLines = overlayLines.slice(0, maxHeight);
}
const { row, col } = this.#resolveOverlayLayout(options, overlayLines.length, termWidth, termHeight);
packages/tui/test/render-stress-harness.ts
+5 -7
View File
@@ -3004,7 +3004,7 @@ function compositeExpectedOverlays(
if (!isExpectedOverlayVisible(entry, termWidth, termHeight)) continue;
const firstLayout = resolveExpectedOverlayLayout(entry.options, 0, termWidth, termHeight);
let overlayLines = entry.component.render(firstLayout.width);
if (firstLayout.maxHeight !== undefined && overlayLines.length > firstLayout.maxHeight) {
if (overlayLines.length > firstLayout.maxHeight) {
overlayLines = overlayLines.slice(0, firstLayout.maxHeight);
}
const layout = resolveExpectedOverlayLayout(entry.options, overlayLines.length, termWidth, termHeight);
@@ -3039,7 +3039,7 @@ export function resolveExpectedOverlayLayout(
overlayHeight: number,
termWidth: number,
termHeight: number,
): { width: number; row: number; col: number; maxHeight: number | undefined } {
): { width: number; row: number; col: number; maxHeight: number } {
const opt = options ?? {};
const margin =
typeof opt.margin === "number"
@@ -3056,11 +3056,9 @@ export function resolveExpectedOverlayLayout(
width = Math.max(width, opt.minWidth);
}
width = Math.max(1, Math.min(width, availWidth));
let maxHeight = parseOverlaySizeValue(opt.maxHeight, termHeight);
if (maxHeight !== undefined) {
maxHeight = Math.max(1, Math.min(maxHeight, availHeight));
}
const effectiveHeight = maxHeight !== undefined ? Math.min(overlayHeight, maxHeight) : overlayHeight;
let maxHeight = parseOverlaySizeValue(opt.maxHeight, termHeight) ?? availHeight;
maxHeight = Math.max(1, Math.min(maxHeight, availHeight));
const effectiveHeight = Math.min(overlayHeight, maxHeight);
let row: number;
let col: number;
if (opt.row !== undefined) {
packages/tui/test/render-stress-oracles.test.ts
+1 -1
View File
@@ -51,7 +51,7 @@ describe("render stress oracle helpers", () => {
width: 10,
row: 6,
col: 29,
maxHeight: undefined,
maxHeight: 8,
});
});