Skip to content

fix(pi-tui): anchor cursor and pin height for inline images#1973

Open
Coiggahou2002 wants to merge 1 commit into
MoonshotAI:mainfrom
Coiggahou2002:fix/pi-tui-inline-image-cursor
Open

fix(pi-tui): anchor cursor and pin height for inline images#1973
Coiggahou2002 wants to merge 1 commit into
MoonshotAI:mainfrom
Coiggahou2002:fix/pi-tui-inline-image-cursor

Conversation

@Coiggahou2002

Copy link
Copy Markdown

Summary

Inline images (pasted screenshots rendered in the transcript) can corrupt the TUI's row accounting: after an image is drawn, the hardware cursor ends up on a different row than the renderer's model expects. Every subsequent differential write then lands on the wrong rows — visible as text fragments overlapping the image area and stale rows that never get cleared, until the next full redraw.

Two independent assumptions in the iTerm2 path cause this, plus one hardening gap in the Kitty path:

Root cause

1. height=auto makes the drawn row count unpredictable (renderImage in packages/pi-tui/src/terminal-image.ts)

The TUI reserves size.rows lines for the image, computed by calculateImageCellSize from pi-tui's cell dimensions (default 9x18 until the terminal answers the cell-size query, and capped by maxHeightCells). But the escape sequence tells the terminal height=auto, so the terminal derives the drawn height from width=<columns> cells and its own cell metrics and aspect handling. These need not agree. Example: a 2000x1514 screenshot capped at maxHeightCells: 12 reserves 12 rows, but with 9x18 cells and width=32 an aspect-derived draw is ceil(32 * 9/18 * 1514/2000) = 13 rows — one row more than reserved.

2. The post-draw cursor position is assumed, not anchored (Image.render in packages/pi-tui/src/components/image.ts)

For iTerm2 the component moves the cursor up to the first reserved row, emits the image, and assumes the cursor comes back to the last reserved row. That assumption holds only when the terminal leaves the cursor on the last drawn row and drawn rows == reserved rows. Real terminals differ: iTerm2 advances the cursor past the drawn image, and some emulators (e.g. SwiftTerm) line-feed once per drawn row, landing one row below the image. Any deviation desyncs every relative cursor move the differential renderer issues afterwards.

3. Kitty C=1 is not honored by all emulators (same file)

The Kitty path relies on C=1 ("don't move the cursor") — spec-correct, and fine on kitty/Ghostty/WezTerm. Emulators that move the cursor during placement anyway break the same accounting.

Fix

  • renderImage now emits height=<size.rows> (cells) instead of height=auto for iTerm2, so the drawn image always fits exactly the rows the TUI reserved (preserveAspectRatio still applies — the image is fit inside the columns x rows box, same as the Kitty c=/r= box).
  • Image.render wraps both the Kitty and the iTerm2 draw in DEC save/restore cursor (ESC 7 / ESC 8): save on the last reserved row, move to the draw position, draw, restore. The cursor deterministically returns to the row the TUI's model expects, regardless of terminal behavior. On spec-compliant terminals this is a no-op.

No other callers of renderImage exist besides the Image component.

Test plan

  • packages/pi-tui suite passes: 712 tests (node --test test/*.test.ts)
  • New tests in packages/pi-tui/test/terminal-image.test.ts:
    • iTerm2 renderImage pins height=<rows>, never height=auto (uses the 2000x1514 / 12-row-cap overflow case)
    • iTerm2 Image.render emits ESC 7 + cursor-up + OSC 1337 + ESC 8 on the last of rows lines, and the wrapped line is still detected by isImageLine
    • single-row iTerm2 images omit the cursor-up but keep the save/restore anchor
  • Existing Kitty placement test updated to expect the save/restore wrapper (regression protection for the Kitty path)
  • apps/kimi-code image tests pass (image-thumbnail, user-message, image-placeholder: 25 tests)
  • tsc --noEmit clean for pi-tui; oxlint --type-aware reports no new warnings on changed files
Before/after: iTerm2 draw line for a 4-row image

Before:

\x1b[3A\x1b]1337;File=inline=1;width=4;height=auto;preserveAspectRatio=1:<b64>\x07

After:

\x1b7\x1b[3A\x1b]1337;File=inline=1;width=4;height=4;preserveAspectRatio=1:<b64>\x07\x1b8

The cursor is now guaranteed to be back on the last reserved row after the draw, and the image cannot exceed its 4 reserved rows.

The iTerm2 path emitted height=auto, so the terminal derived the drawn
row count from its own cell metrics while the TUI reserved rows from its
own (possibly default or capped) cell dimensions. Any mismatch left the
hardware cursor on a different row than the TUI's model after the draw,
and every subsequent differential write landed on the wrong rows —
rendered as text fragments overlapping the image or stale rows that are
never cleared. The same class of desync hits terminals that move the
cursor during image placement despite Kitty's C=1.

- renderImage() now pins height=<reserved rows> cells for iTerm2 so the
  drawn image always fits the rows the TUI accounted for.
- Image component wraps both Kitty and iTerm2 draws in DEC save/restore
  cursor (ESC 7 / ESC 8), so the cursor deterministically returns to the
  expected row no matter where the terminal leaves it after drawing.
  On spec-compliant terminals this is a no-op.
@changeset-bot

changeset-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 183f7ec

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant