[ogulcancelik]

what was wrong

scrolling an agent UI with images (pi, alt screen) hung. when an image scrolled out of view herdr deleted it from the outer terminal, so every time it scrolled back herdr re-sent the full pixels server → client → terminal. pi itself only sends tiny "place image N here" commands and keeps up to 16 hidden images loaded, so outside herdr the same scroll is instant.

what this does

• the server fills the existing retained_assets scene field with recently hidden images whose current pane image still matches the delivered fingerprint (deleted or replaced images drop out). the client already keeps retained images loaded, so no wire or client change. bounded like pi's own cache: 16 images / 64 MiB decoded, most recently visible first.
• keep image fingerprints while the image exists, so an image returning to view isn't re-hashed (~1.1 ms per 8 MB image with the pane lock held).
• small: read scroll offset once per pane instead of per placement.

numbers

pi-style scroller, 12 images, full sweep down and back (bytes herdr writes to the terminal):

	before	after
first sweep	6.9 MB	3.5 MB (only never-seen images)
second sweep	6.9 MB	60 KB
image deletes sent to terminal	65	0

retained text update (render_scale_profile_retained_graphics, new offscreen case):

	no image	visible image	hidden images
1 pane	8 µs	130 µs	130 µs
15 panes	37 µs	169 µs	161 µs

hidden images cost what a visible image already costs on master: clients reject row patches while retained_assets is non-empty, so text updates keep going out as surface messages. making that cheap needs a negotiated client capability, left for a follow-up.

tests

• hide / show again without re-send / delete while hidden / reuse id for new pixels
• eviction order past the cap
• replacing a hidden image with same-sized pixels drops the stale copy
• text updates with only hidden images arrive as surfaces the client accepts, keep the hidden image, and don't resend it
• fingerprint survives hidden placements and is dropped when the image is deleted

manually checked with pi, terminal-browser, yazi, and kitten icat in a fresh session.

not in this pr

• png pass-through stays off (deliberate, corrupt-png validation)
• visible images still take the graphics refresh path on text updates (unchanged from master)
• herdr re-places unmoved visible images each frame because pi's a=p has no stable p=; cheapest fix is on pi's side

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: acd2f103-2fb7-4031-b32b-7172096f0929

📥 Commits

Reviewing files that changed from the base of the PR and between 7cc71db and 8f97e8f.

📒 Files selected for processing (6)

• crates/ghostty-vt/src/lib.rs
• src/kitty_graphics/surface.rs
• src/pane.rs
• src/pane/terminal.rs
• src/server/headless/tests/pane_graphics.rs
• src/terminal/runtime.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds Kitty image fingerprint lookup through the terminal and runtime layers. Fingerprint cache entries remain available while their images are stored, including when hidden. Scene collection uses fingerprints to validate offscreen images and retains eligible assets within limits of 16 images and 64 MiB. Headless tests cover reuse, replacement, deletion, deferred delivery, and eviction.

Suggested reviewers: jjliebig

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 8f97e

Offscreen image retention follows the intended eviction order and decoded-memory limit. No merge-blocking issue was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8f97e

Previously delivered images can now remain loaded while offscreen. Pane ownership, image-identity checks, and retention limits constrain the change, and no security vulnerability was verified. Memory accounting and reconnect behavior are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Terminal-produced images can now occupy client terminal image state after leaving view, but retention is selected per collected scene from pane-terminal images in its current workspace. The evidence does not establish a cross-client aggregate memory bound.

Trust Boundaries and Controls

• observed — Retention crosses from pane terminal state to client scene ownership only after current-pane mapping and fingerprint checks. The client continues to reconcile retained assets against the desired scene rather than trusting retention indefinitely.

Resilience and Maintainability Implications

• inferred — The count and data_len limits constrain retained entries, while fingerprint mismatch and missing pane runtime cause candidates to be dropped. Whether data_len bounds decoded memory for every supported image format remains unverified.

Hardening Proposals

• proposed — Confirm that retention accounting bounds actual client-resident memory for PNG as well as raw image formats, and assess the combined bound when several clients view the same image-producing pane.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 8 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: retaining offscreen Kitty images to avoid resending them during scrolling.
Description check	✅ Passed	The description explains the scrolling issue, the retention behavior, bounds, tests, performance results, and out-of-scope items. It directly matches the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kangal-bot]

@coderabbitai review
@greptileai

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Changes how offscreen kitty graphics are cached and delivered.

The PR appears safe to merge; no outstanding findings or new actionable issues remain.

Summary

The PR retains recently hidden Kitty images so scrolling them back into view does not resend their pixels. It also preserves fingerprints for stored, unplaced images and restores full surface updates for text changes while images are retained.

• Both previous findings are addressed: hidden-image text updates use a surface message, and retention checks the current image fingerprint rather than dimensions alone.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Visible image delivered] --> B[Placement leaves view]
  B --> C{Stored image fingerprint matches?}
  C -- Yes, within limits --> D[Retain client asset]
  D --> E[Placement returns without pixel resend]
  C -- No --> F[Release stale asset]

Loading

_{Reviews (2) · Last reviewed commit: "fix: keep offscreen kitty images loaded ..."}

[ogulcancelik]

@coderabbitai review
@greptileai

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.