fix(tui): cap inline image height and preserve multiplexer scrollback#587
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81e4034cfb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
81e4034 to
ce08ddf
Compare
Inline images (screenshots from puppeteer, assistant images) rendered at unbounded height — a 1280x800 screenshot consumed ~32 terminal rows, causing excessive scrolling. In multiplexers (tmux, screen, zellij), this was compounded by fullRender clearing scrollback on every resize. Image height cap: - New setting tui.maxInlineImageRows (default 20) - Effective cap: min(setting, floor(viewport * 0.6)) - Shared resolveImageOptions() replaces per-site settings reads - Existing calculateImageFit() already handles maxHeightCells Multiplexer scrollback preservation: - Skip CSI 3J (clear scrollback) in tmux/screen/zellij - Skip fullRender(true) on height change in multiplexers - Cached isMultiplexer constant (TMUX, STY, ZELLIJ env vars)
ce08ddf to
2f21637
Compare
Verification ResultsTested the fix against 4 screenshot types x 4 terminal sizes. All 16 scenarios pass height cap constraints, plus scrollback preservation confirmed in tmux. Worst case was a mobile viewport screenshot (390x844) on a 24-row terminal: 109 rows down to 14 (92% reduction). Portrait screenshots were the next worst at 80 -> 14. Edge cases verified
|
# Conflicts: # packages/coding-agent/CHANGELOG.md
# Conflicts: # packages/coding-agent/CHANGELOG.md
What
Cap inline image height to prevent excessive scrolling, and preserve scrollback history in terminal multiplexers (tmux, screen, zellij).
Image height cap
Inline images rendered at unbounded height. A typical 1280x800 puppeteer screenshot consumed ~32 terminal rows, causing a massive scroll burst via
\r\n.repeat(scroll)indoRender().The
ImageOptions.maxHeightCellsparameter existed but was never passed by either call site:tool-execution.ts:534-- tool result imagesassistant-message.ts:79-- assistant message imagesBoth only passed
maxWidthCellsviasettings.get("tui.maxInlineImageColumns"). Height was an emergent property of (width cap x aspect ratio), never validated against viewport.Changes:
tui.maxInlineImageRows(default: 20, set 0 for viewport-only limit)resolveImageOptions()computes effective cap asmin(setting, floor(viewport * 0.6))resolveImageOptions()instead of reading width-only settingscalculateImageFit()already handlesmaxHeightCellscorrectly -- no TUI package changes neededEffective behavior by terminal size:
Multiplexer scrollback preservation
fullRender(true)emits\x1b[3J(CSI 3J) which clears the entire scrollback buffer. This fired on every terminal height change (line 1060). In multiplexers, height changes happen on pane zoom, split, and resize -- destroying scrollback that users actively navigate.There was already a Termux exception for the same class of problem (soft keyboard height toggle).
Changes:
isMultiplexerconstant detecting tmux (TMUX), GNU screen (STY), and zellij (ZELLIJ)\x1b[2J\x1b[H) is sufficient for re-renderingfullRender(true)on height change in multiplexers -- same pattern as existing Termux exceptionWhy
Puppeteer screenshots caused 30+ lines of scroll per image. In tmux, this was compounded by scrollback being wiped on every pane resize. The combination made puppeteer unusable in tmux and disruptive everywhere else.
Root cause analysis:
maxHeightCellsexisted in the API but was never wired to callers (design oversight -- width was capped but height was not)Testing
bun check:tspasses (biome + tsgo)bun checkpasses