fix(images): correct show_image sizing/crop across three layers (#323, #324, #325)

[kshivang]

Fixes the three show_image / OSC 1337 inline-image bugs filed together — each lives in a different layer of the pipeline, so all three are needed for images to render correctly.

#323 — Auto+Auto sizing never clamped height

ImageDimensionCalculator.calculate()'s Auto+Auto branch only scaled down when the image was wider than the terminal. A landscape image narrower than the pane but taller than it (e.g. 960×560 in a wide-but-short horizontal split) rendered at full intrinsic size and clipped vertically.

Fix: fit by the most-constrained axis — compute width and height scale factors, apply min(widthScale, heightScale), the same approach the "both explicitly specified" branch already used.

#324 — fresh panes sized against the placeholder 80×24 @ 10×20px grid

awaitPromptReady() waited only for the shell prompt (OSC 133;A). Real cell metrics and the real grid arrive via an independent Compose layout path with no ordering guarantee, and the prompt usually wins the race — so images (including explicit % specs) were sized against terminalWidthPx = 800, terminalHeightPx = 480 regardless of actual pane size.

Fix:

• BossTerminal gains a one-way isUiLayoutReady latch (markUiLayoutReady()).
• ProperTerminal's initial layout resize (in onGloballyPositioned) now pushes setCellDimensions(cellWidth, cellHeight) explicitly and sets the latch — the pre-existing LaunchedEffect push has no ordering guarantee relative to the layout callback.
• The MCP server's wait (renamed awaitPaneReady) awaits both prompt readiness and the layout latch under the same timeout.

#325 — baked cell footprint permanently cropped images

ImageCell.totalCellsX/Y is a one-time snapshot of the grid at insertion, but TerminalCanvasRenderer iterates col < visibleCols computed live per frame. If the baked footprint exceeded the live width, the columns past the pane edge were simply never visited — a crisp, hard, permanent crop (the buffer footprint is never revisited, so it didn't self-heal even after the grid settled).

Fix: re-fit at draw time. The renderer derives the anchor column (col - cellX) and the columns actually reachable this frame; if the baked span exceeds that, the full bitmap is sliced across the available columns with the row count shrunk proportionally to preserve aspect ratio. Self-healing: if the pane widens again, the image returns to its original span.

Side finding confirming the issue's suspicion: ImageRenderer.renderImages() has zero call sites — the per-cell path in TerminalCanvasRenderer is the only live inline-image renderer. Left in place to keep this PR focused.

Tests

New ImageDimensionCalculatorTest (compose-ui desktopTest): the #323 regression case (960×560 in a 1000×400px pane), width-only overflow, both-axis overflow, fits-unchanged, and explicit-percent sanity — 5/5 green. All touched modules compile.

Manual check suggested before release: show_image with a landscape PNG into a fresh horizontal_split should now come out full-size and uncropped.

Fixes #323
Fixes #324
Fixes #325

🤖 Generated with Claude Code

[claude]

Code Review — PR #326: show_image sizing/crop fixes

Reviewed all three fixes against the source. This is a clean, well-scoped PR — each change targets a distinct layer, the diagnosis in the description matches the code, and I verified the supporting claims (ImageRenderer.renderImages() genuinely has ZERO call sites; ImageCell.cellX/cellY/totalCellsX/Y mean what the re-fit math assumes). 👍

What is correct

323 — Auto+Auto axis clamping. The min(widthScale, heightScale) approach correctly fits by the most-constrained axis and mirrors the pre-existing "both specified" branch. The refactor is behavior-preserving for the old width-only path (targetWidthPx * widthScale == terminalWidthPx modulo rounding) while now also handling height/both. Good.

325 — draw-time re-fit. The logic is sound. I traced the invariants:

• anchorCol = col - cellX is constant across a row for a given image, and availableCols is always > 0 for any visited cell (col < visibleCols implies anchorCol < visibleCols), so coerceAtLeast(1) is just belt-and-suspenders.
• In the shrink case the full bitmap is compressed across effCellsX = availableCols columns, so the whole image is shown (smaller), not cropped — and it self-heals on widen. Correct.
• effCellsY rounding via (a*b + d/2)/d is round-to-nearest; the cellY < effCellsY guard correctly blanks the now-unused bottom rows.

324 — layout-ready latch. One-way @volatile latch pushed from the initial onGloballyPositioned resize (guarded by hasPerformedInitialResize), awaited alongside the OSC 133;A signal under a shared timeout. The explicit setCellDimensions push in the layout callback closes the ordering race with the LaunchedEffect. Reasonable design.

Minor observations (non-blocking)

1. Horizontal overflow also shrinks vertically. When only width overflows the live pane, the image is compressed on both axes (to preserve aspect), leaving blank buffer rows below the re-fit footprint where the baked totalCellsY still reserves space. This is the right call vs. cropping/stretching, but worth a one-line comment noting the intentional gap so a future reader does not "fix" it.

2. Pure vertical shrink at draw time is not re-fit. The re-fit only triggers on totalCellsX > availableCols. If a pane loses rows after insertion (image taller than the live grid), the bottom is still cropped by the per-line loop. Likely out of scope (images scroll), but not covered by 323 either since that is sizing-time only — flagging in case it surfaces later.

3. Test coverage is 323-only. The 5 new tests are solid and the 323 regression case (960x560 in 1000x400) is exactly right. But the two trickier fixes — the renderer re-fit (325) and the latch race (324) — have no automated coverage. The re-fit is pure integer math over (totalCellsX, totalCellsY, cellX, cellY, visibleCols) and could be extracted into a small testable helper (e.g. computeEffectiveCells(...)) returning (effCellsX, effCellsY), which would let you assert compress/self-heal/edge behaviors without a Compose harness. Worth considering.

4. Dead code. ImageRenderer.renderImages() (confirmed unused) — good call leaving it out of this PR to stay focused; suggest a follow-up issue to remove it so it does not rot.

Verdict

No correctness or security concerns found; the per-cell re-fit adds only a few integer ops per image cell per frame (negligible). Nice work — approve pending the maintainer view on extracting the re-fit helper for testability.

🤖 Generated with Claude Code

[kshivang]

Follow-up commit aea1379 after field-testing on the dev build: the #324 readiness gate was unreliable for freshly created tabs — images intermittently baked into a ~2-row squashed footprint (full width, 2 rows tall) or vanished into scrollback.

Causes fixed:

1. The uiLayoutReady latch was guarded by hasPerformedInitialResize, a composition-scoped remember — and ProperTerminal's composition is reused across tab switches, so the latch never fired for any tab created after the first. Now guarded by the terminal's own latch.
2. A fresh pane's first layout pass can transiently measure a near-zero canvas (rows clamped to 2); awaitPaneReady now requires the grid to hold still for 3×60ms samples and be ≥10×3 before trusting it.
3. Layout settle no longer shares the 1.5s shell-prompt budget (a slow shell rc could exhaust it, expiring the gate mid-settle); it has its own 3s budget.

Verification: 6/6 consecutive fresh-tab show_image placements on the dev build baked full-size (17–21 row) footprints, measured programmatically via read_scrollback; previously this failed roughly every other run.

[claude]

Review — PR #326: show_image sizing/crop fixes

Thorough, well-scoped PR. Each of the three fixes targets a genuinely distinct layer, the commit messages and inline comments are excellent (they explain the why and the race conditions clearly), and the follow-up commit shows real field validation. Overall this looks correct and ready. A few notes below.

Correctness — looks good

• show_image: Auto+Auto sizing only clamps to terminal WIDTH, never HEIGHT — landscape images clip vertically in short split panes #323 (ImageDimensionCalculator): The min(widthScale, heightScale) fit is correct and now mirrors the existing "both specified" branch. Verified the show_image: Auto+Auto sizing only clamps to terminal WIDTH, never HEIGHT — landscape images clip vertically in short split panes #323 case (960x560 in 1000x400px): heightScale ~= 0.714 dominates -> 685x400, cellHeight lands at exactly 20 rows. Aspect preserved.
• Inline image cell footprint (totalCellsX/Y) is baked in at insertion time and permanently crops if it exceeds the live visibleCols/visibleRows at draw time #325 (renderer re-fit): The per-cell effCellsX/effCellsY derivation is consistent across every cell of an image (anchorCol = col - cellX is invariant), rows shrink proportionally so pixel aspect is preserved (cell aspect is already baked into the original span), and it self-heals when the pane widens since totalCellsX <= availableCols restores the full span. Integer rounding on effCellsY is sound.
• show_image on freshly-created panes/tabs sizes against placeholder 80x24@10x20px grid, not the real pane — images render far too small #324 (readiness gate): The reused-composition insight (keying on terminal.isUiLayoutReady rather than the composition-scoped hasPerformedInitialResize) is the crux and is handled correctly. The dedicated LAYOUT_SETTLE_TIMEOUT_MS budget and the sanity floor (MIN_SANE_GRID_*) are reasonable guards against the transient 2-row grid. No deadlock risk: if the latch never sets (e.g. a sub-10px pane), withTimeoutOrNull falls through and injection proceeds anyway.

Suggestions (non-blocking)

1. Test coverage is uneven. show_image: Auto+Auto sizing only clamps to terminal WIDTH, never HEIGHT — landscape images clip vertically in short split panes #323 gets solid unit tests, but the Inline image cell footprint (totalCellsX/Y) is baked in at insertion time and permanently crops if it exceeds the live visibleCols/visibleRows at draw time #325 re-fit math and the show_image on freshly-created panes/tabs sizes against placeholder 80x24@10x20px grid, not the real pane — images render far too small #324 gate are the more subtle fixes and have none. The renderer arithmetic (effCellsX/effCellsY derivation) is a pure integer function — worth extracting into a small helper so the width-overflow / self-heal / rounding cases can be asserted without a live DrawScope. That is where a future regression is most likely to slip in unnoticed.

2. Sequential waits are additive. awaitPaneReady waits up to timeoutMs (default 1.5s) for the prompt, then up to 3s for layout settle — worst case ~4.5s of a hung tool call before injection proceeds anyway. Probably fine in practice, but the two conditions are independent and could be awaited concurrently to cap worst-case latency. Also note a genuinely tiny pane (below 10 cols / 3 rows) will always burn the full 3s settle budget, as the comment acknowledges.

3. Blank reserved rows below re-fit images. When effCellsY < totalCellsY, the image draws smaller but the buffer footprint still reserves totalCellsY rows, leaving blank cells below it (text still starts after the full footprint). Correct for preserving aspect, just worth being aware of as a visible behavior — the image shrinks and leaves a gap rather than reflowing.

4. Minor: the imageCell.cellX < effCellsX guard is always true for visited cells (the loop stops at col < visibleCols, and anchorCol + effCellsX == visibleCols when shrunk), so only the cellY < effCellsY half does real work. Harmless/defensive, just redundant.

5. Good call leaving the dead ImageRenderer.renderImages() out of scope and just flagging it — agree it should not ride along in this PR.

Nice work — the diagnosis quality here is high.

Generated with Claude Code

[kshivang]

Second follow-up (a455207): the gate alone was still flaky under interactive layout churn — it samples the grid before injection, but a transient degenerate resize (rows clamped to 2 by a near-zero layout measure) could land in the TOCTOU gap between the gate passing and the emulator processing the OSC. Three layers added, the last one airtight:

1. ProperTerminal refuses to apply sub-4×3 resizes (settling artifacts, not panes) and only arms the readiness latch off a sane measure.
2. TabbedTerminal's background-tab grid propagation waits for the active tab's real layout and never fans out degenerate grids — a fresh active tab used to broadcast its placeholder 80×24 (and any transient blip) to every background tab.
3. BossTerminal tracks the last sane grid across resizes and processInlineImage falls back to it whenever the live grid is degenerate at insertion — same thread-serialized state the calculator reads, so no resize source can poison sizing regardless of timing.

Verified: 8/8 back-to-back fresh-tab placements with zero settle time bake full-size footprints; the 2-row squash no longer reproduces. Separately filed #327 for an unrelated pre-existing race found while stress-testing (concurrent new-tab creations can leave a shell that never starts).

[claude]

Review: fix(images): correct show_image sizing/crop across three layers

Thorough, well-documented PR — the comments mapping each layer of the pipeline to its issue made this easy to follow, and the defense-in-depth approach (skip degenerate resize → wait for stable grid → fall back to last-sane grid → re-fit at draw time) is sensible. The #323 calculator fix is clean and correct, and shipping it with a focused pure-function unit test is exactly right. Verified signatures/types compile-wise (tab.terminal is concrete BossTerminal, DimensionSpec.Percent(value) matches, no key() around ProperTerminal so the "composition reused across tabs" reasoning behind the terminal-scoped latch holds). I did not run ./gradlew build.

A few things worth considering:

1. Residual vertical crop is not addressed (symmetric to #325)

The draw-time re-fit in TerminalCanvasRenderer only triggers on width overflow (imageCell.totalCellsX > availableCols), shrinking rows only as a consequence. The renderer still iterates row in 0 until visibleRows, so if a pane shrinks vertically after an image is inserted (e.g. dragging a horizontal split smaller), rows past visibleRows are simply never visited — the same hard, permanent bottom-crop that #325 fixed horizontally. Worth at least a follow-up note; the fix is arguably symmetric to what you already did for columns.

2. The "degenerate grid" threshold is encoded in 4 places with 2 different values

• BossTerminal: MIN_SANE_GRID_COLS = 10, MIN_SANE_GRID_ROWS = 3
• BossTermMcpServer: MIN_SANE_GRID_COLS = 10, MIN_SANE_GRID_ROWS = 3 (duplicated)
• ProperTerminal: inline newCols >= 4 && newRows >= 3
• TabbedTerminal: inline size.columns < 4 || size.rows < 3

Two copies of the exact same constants plus two inline 4/3 literals invites drift. Consider a single shared source of truth (or at minimum a comment cross-referencing them).

3. Mismatch between the latch threshold (>=4 cols) and the "sane" threshold (>=10 cols)

markUiLayoutReady() latches at newCols >= 4, but awaitPaneReady's stability loop only counts a sample when width >= 10. So a genuinely narrow pane (5-9 cols) latches "UI ready" instantly yet its stability check never sees sane == true — it always burns the full LAYOUT_SETTLE_TIMEOUT_MS (3s) before proceeding. Combined with the sequential shellReadyTimeoutMs + 3s budget, fresh show_image calls into small/never-sane panes can now block noticeably longer than before. Correctness is preserved, but the two thresholds should probably be reconciled or the relationship documented.

4. Possible behavior change: a remote active tab no longer propagates its grid

The new if (!activeTerminal.isUiLayoutReady) return@collect guard in TabbedTerminal — remote tabs return early in onGloballyPositioned and never call markUiLayoutReady(), so isUiLayoutReady is permanently false for them. When a remote tab is the active one, the window->background-tab grid propagation (which previously ran) is now silently skipped. If that broadcast-from-remote-active path was intentional before, this is a regression; if it was never desired, a one-line comment would make the intent explicit.

Minor

• effCellsX/effCellsY are recomputed for every image cell every frame in the hot draw loop; they're constant per row (same anchorCol). Cheap integer math, but hoistable.
• ImageDimensionCalculatorTest lives in compose-ui/desktopTest while the class under test is in bossterm-core-mpp — works via the dependency, but the core module's test source set would be the more natural home.
• The grid-stability loop reads session.textBuffer.width/height lock-free while ProperTerminal mutates it on another thread — benign for sampling (worst case is one extra iteration), just noting it.

Nice work overall — the core fixes are correct and the reasoning is unusually well captured in the code comments.

🤖 Generated with Claude Code

[kshivang]

Third follow-up (d210803), this time verified at the pixel level — driving the MCP against the dev build and screenshotting the actual window, not just checking buffer contents:

New fix — HiDPI half-size images: ImageDimensionCalculator receives cell metrics in device px (16.86×35 on 2×), but intrinsic image sizes and px specs are logical quantities — so auto-sized images baked at half their intended size on retina. The calculator now takes a pixelScale (pushed from LocalDensity alongside cell metrics); Auto/Pixels specs scale by it, percent/cells are unit-free and unchanged. Verified: an auto+auto 960×560 image now spans the full 73-col pane (1230 device px) vs 57 cols before.

Permanent flight recorder: one INFO line per image insertion (grid, metrics, scale, computed cells) in BossTerminal, plus a deduped IMG-REFIT line in the renderer whenever draw-time geometry disagrees with a baked footprint — the exact state behind the earlier silent corruption. Healthy frames log nothing; the next field report of a mangled image comes with numbers.

Verification summary on the current build: all insertions across the full test-script run + 7 fresh-tab placements sized against real grids (73×30); renderer drew every image row with eff == total; 5/5 captured fresh-tab runs pixel-complete (automated red-frame bounding-box analysis of window screenshots); the earlier 2-row squash did not reproduce.

[claude]

Review — fix(images): correct show_image sizing/crop across three layers

Thorough, well-diagnosed work. The commit messages and PR body read as a genuine debugging log across four passes, and the layering of the three root causes (calculator math → readiness/ordering → draw-time re-fit) is convincing. The core fixes look correct:

• show_image: Auto+Auto sizing only clamps to terminal WIDTH, never HEIGHT — landscape images clip vertically in short split panes #323 min(widthScale, heightScale) in the Auto+Auto branch is the right fit-by-most-constrained-axis approach and now matches the "both specified" branch. Good.
• show_image on freshly-created panes/tabs sizes against placeholder 80x24@10x20px grid, not the real pane — images render far too small #324 latch keyed on terminal.isUiLayoutReady rather than the composition-scoped hasPerformedInitialResize is a real bug fix — reused compositions across tab switches would have left later tabs never latching. The follow-up commits (grid-stability sampling, separate layout-settle budget, degenerate-grid floors) are each independently justified.
• Inline image cell footprint (totalCellsX/Y) is baked in at insertion time and permanently crops if it exceeds the live visibleCols/visibleRows at draw time #325 draw-time re-fit correctly turns a permanent hard crop into a self-healing compress, and the aspect-preserving row shrink is sound.
• HiDPI pixelScale — interpreting intrinsic/Pixels specs as logical px against device-px cell metrics is the correct fix, and the "Default 1f keeps 1x byte-identical" invariant is preserved. The unit reasoning checks out across all four spec branches.

Things worth addressing

1. println("IMG-REFIT ...") in the render hot path (TerminalCanvasRenderer.kt:806). This writes raw stdout from inside the per-cell draw loop. Even deduped, stdout println is a poor fit — the rest of core uses SLF4J (LOG in BossTerminal). Suggest routing through a logger at DEBUG, or gating behind a debug flag. As written it's a permanent user-facing stdout line the first time any image re-fits.

2. imageDiagSeen grows unbounded (TerminalCanvasRenderer.kt:309). A static set keyed on a string containing imageId, lineIdx, col, visCols, etc. Every distinct geometry/line/scroll position adds a permanent entry that never clears for the process lifetime — an ever-growing static on a long-lived object. Consider a bounded cache, clearing it, or just dropping it alongside the logger change.

3. Raised resize floor to 4×3 (ProperTerminal.kt:499) and fan-out floor (TabbedTerminal.kt:799). Reasonable against transient degenerate measures, but a genuinely tiny pane (e.g. a 2-row bottom split) will now never resize below the previous grid, leaving the PTY with a stale larger row count. MIN_ROWS is 2, so this is a behavior change for the 2–3 row case. Low severity, but worth an inline note acknowledging the tradeoff.

4. awaitPaneReady latency for legitimately tiny panes. If a pane is genuinely < 10 cols or < 3 rows, the sane check never passes, stableSamples never increments, and a fresh-pane show_image waits the full LAYOUT_SETTLE_TIMEOUT_MS (3s) before falling through. The comment acknowledges this and it only affects fresh panes, so acceptable — just flagging the fixed 3s stall.

5. Cosmetic gap from row shrink in re-fit. When the draw-time re-fit shrinks effCellsY < totalCellsY, the image occupies fewer rows visually while the buffer footprint still reserves totalCellsY rows, leaving blank rows between the image and following content until the pane widens. Not a correctness bug (self-heals), just the expected visual result.

Test coverage

ImageDimensionCalculatorTest is a solid, well-targeted addition — #323 regression, both-axis overflow, retina/logical-size, and percent sanity. Understandable gaps: the draw-time re-fit math in TerminalCanvasRenderer and the awaitPaneReady stability gate are layout/thread-bound and rely on the manual field verification in the commits. The effCellsY proportional-shrink formula in particular is pure integer arithmetic that could be extracted and unit-tested as a regression guard on #325.

Minor

• ImageRenderer.renderImages() confirmed dead (zero call sites) — good call keeping it out of scope; consider a follow-up so it doesn't rot.
• The per-insertion LOG.info in processInlineImage is fine (fires per image, not per frame), though INFO-per-image may be noisier than intended for a shipped build — DEBUG might be a better home.

Overall: correct, well-reasoned fixes for all three issues. My only real ask before merge is trimming/relocating the stdout println + unbounded static set (#1, #2); the rest are notes.

Reviewed by Claude — automated PR review.

[kshivang]

Root cause of all the residual flakiness found, reproduced deterministically, and fixed (fb68b47).

The failing pattern was never about insertion timing: an image renders pixel-perfect, then the first pane/window resize after insertion permanently corrupts it. Reproduced on demand: place image → complete (876×455 px) → split the pane → 590×32 px garbage strip. When a settling resize lands within the same second as placement (fresh tabs/splits), it looks like the image "never rendered" — hence every earlier report of intermittent strips and missing images.

Mechanism: ChangeWidthOperation reflows ImageCells like text characters (placeImageCell() wraps mid-image onto continuation lines). Each cell encodes its image-grid position (cellX/cellY vs totalCellsX/Y) and the renderer derives the anchor from col - cellX, so rewrapping scatters the geometry — baked into the buffer forever.

Fix: image-bearing rows pass through width reflow atomically (deep-copied, never rewrapped). Overflow cells simply don't draw, and the draw-time re-fit from the #325 fix compresses the visible portion to the live pane; when the pane widens again the intact cells render at full size. Clipping is recoverable — corruption was not.

Verified via automated window capture + pixel analysis on the dev build: the same place-then-split sequence now yields a compressed-but-complete image (843×455, all corners present). Full compose-ui desktopTest suite green (389 tests).

[claude]

Review: fix(images) — show_image sizing/crop across layers (#323, #324, #325)

Thorough, well-documented work. The three root causes are correctly identified and the comments explaining why each fix is needed (especially the ordering-race and the "corruption is permanent, clipping is not" reasoning) are excellent. The commit-by-commit narrative of field-testing findings is genuinely useful.

A few things worth addressing before merge:

🔴 Dead code left behind in ChangeWidthOperation.kt

The new atomic pass-through returns early whenever line.hasImageCells():
```kotlin
if (line.hasImageCells()) {
myCurrentLine = null; myCurrentLineLength = 0
myAllLines.add(line.copy()); return
}
// Get image cells sorted by column
val imageCells = if (line.hasImageCells()) { ... } else { sortedMapOf() }
```
Everything below that guard that dealt with image cells is now unreachable: imageCells is always empty, so the placeImageCell calls (lines 245, 283) can never fire for image lines — placeImageCell is now dead code entirely. Recommend deleting the old interleave/placeImageCell path and the redundant second hasImageCells() check so the file doesn't mislead future readers into thinking both paths are live.

🟡 println on the render hot path + unbounded dedup set

In TerminalCanvasRenderer, imageDiagSeen is a static ConcurrentHashMap.newKeySet and if (imageDiagSeen.add(sig)) println("IMG-REFIT $sig") runs in the draw loop. Two concerns for a shipping build:

• println goes to stdout from the draw loop — the rest of the codebase uses the LOG facility (as you do in processInlineImage). Prefer LOG.info/debug for consistency and control.
• imageDiagSeen never clears. The signature embeds col, visCols, anchorCol, eff=(...), so every distinct geometry (each resize width, each column) adds a permanent entry for the process lifetime — bounded in practice but grows with resize churn. Consider gating this "flight recorder" behind a debug flag or bounding/clearing the set.

The per-insertion LOG.info in processInlineImage is fine but fires on every image at INFO — debug might be a better home.

🟡 Raised layout floor (2 → 4 cols / 3 rows)

ProperTerminal.onGloballyPositioned, the TabbedTerminal fan-out, and the MCP MIN_SANE_GRID_* now all treat anything below 4×3 as a transient artifact. Sensible for filtering the near-zero first-pass measure, but a legitimately tiny pane (e.g. a 2-row horizontal split) will never apply its real resize or set isUiLayoutReady — so show_image there falls through to the LAYOUT_SETTLE_TIMEOUT_MS (3s) fallback and sizes against placeholders/last-sane. Worth confirming no real workflow produces a ≤2-row pane; if it can, this is a (documented) regression for that case.

🟡 Worst-case awaitPaneReady latency

The prompt wait and the layout-settle wait are now sequential with independent budgets, so worst case is timeoutMs + LAYOUT_SETTLE_TIMEOUT_MS (~1.5s + 3s ≈ 4.5s) for a fresh pane whose shell never emits OSC 133;A and whose layout never reaches sane dims. Fine for the happy path (~200–500ms as noted), just flagging the tail.

🟢 Correctness of the sizing math — looks right

• The Auto+Auto min(widthScale, heightScale) fix is correct and aspect-preserving (same scale applied to both axes); the show_image: Auto+Auto sizing only clamps to terminal WIDTH, never HEIGHT — landscape images clip vertically in short split panes #323 regression test confirms it.
• HiDPI scaling is applied consistently — Percent/Cells stay unit-free, Auto/Pixels and the aspect-fit ratios all scale, so device-vs-device ratios stay valid.
• The draw-time re-fit compresses the full bitmap into reachable columns with rows shrunk proportionally, and the cellX < effCellsX && cellY < effCellsY guard cleanly skips out-of-footprint cells.

🟡 Test coverage gaps

ImageDimensionCalculatorTest covers the calculator well (including the retina case). But the three trickiest new behaviors are untested:

1. BossTerminal.processInlineImage sane-grid fallback — the "size against last-sane grid when live grid is degenerate" logic is unit-testable and is the airtight defense the PR leans on.
2. ChangeWidthOperation atomic image-row pass-through — a regression test that an image-bearing line survives a width change without rewrapping would lock in the Inline image cell footprint (totalCellsX/Y) is baked in at insertion time and permanently crops if it exceeds the live visibleCols/visibleRows at draw time #325-followup fix.
3. Draw-time re-fit in the renderer is harder to unit test, understandably left to manual verification.

Overall this is solid, and the layered defense-in-depth is the right call for a race this slippery. The dead-code removal (🔴) and the println/unbounded-set cleanup (🟡) are the main things I'd want fixed before merge; the rest are judgment calls worth a second look.

🤖 Generated with Claude Code