[akbash-bot]

Issue

Running many agents that redraw their terminals can make the Linux server burn several CPU cores. Snapshot requests then intermittently take seconds.

Problem

Reading terminal text allocated a separate grapheme vector for each populated cell. Concurrent reads fought over musl's allocator lock before assembling the same text into strings.

How did we fix it?

Herdr now reuses one grapheme buffer while reading each row. Text normalization and detection behavior stay unchanged. Consumers that need owned cell snapshots keep their existing API.

Verification

The reported agent-free workload reproduced on unchanged master: 24 working 330×119 panes used 2.78–3.24 cores, with snapshot calls reaching 1.76 seconds. Matched fixed runs used 0.336–0.338 cores; all 30 snapshot calls took 6–12 ms. Output throughput increased and all panes remained working. Both sessions were cleaned up; remote reconnect recovery remains unverified.

Tests protect cell equivalence, row allocation scaling, and tight owned-cell capacity. just check passed 3,898 Rust tests and its other checks, including Windows cross-lint. Six render-scale profiles passed; the native-file profile could not complete because this host lacks required CoW/reflink support.

refs #4506

[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: 87806e9f-24b8-4e4e-af0b-fc9d0da1e03f

📥 Commits

Reviewing files that changed from the base of the PR and between 18b1695 and 7a5574a.

📒 Files selected for processing (1)

• crates/ghostty-vt/src/lib.rs

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

---

📝 Walkthrough

Walkthrough

The terminal library adds for_each_screen_row_cell, which visits row cells using borrowed grapheme slices and a reused buffer. The pane renderer uses this method while preserving its existing cell-rendering rules. Tests compare borrowed traversal with owned rows and check allocation counts for ASCII rows.

Suggested reviewers: ogulcancelik

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 7a557

The change reuses grapheme storage for row reads while preserving pane text handling. The supplied evidence identifies no actionable merge blocker.

Architecture Summary

Architecture risk: 🔵 Low · up to 7a557

The change affects 2 systems.

Changed systems: crates, src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — crates (service) was modified; 2 changed files map to changed impact.
• observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in crates/ghostty-vt/src/test_allocations.rs: Adds a thread-local allocation counter and a global allocator that forwards allocation, zeroed-allocation, reallocation, and deallocation to System, recording only the first three when counting is active on the calling thread. The public count function enables counting for one operation, rejects nested activation, returns the operation result and count, and uses a drop guard to disable counting on exit.
• observed — Modified behavior in src/pane/terminal.rs: ghostty_screen_row replaces row retrieval through screen_text_rows_range and owned-cell iteration with for_each_screen_row_cell; it processes each cell in the callback, skips spacer tails, and preserves the existing blank-cell and Kitty-placeholder handling.
• observed — Modified behavior in crates/ghostty-vt/src/lib.rs: Terminal::for_each_screen_row_cell returns without visiting when row is outside total_rows; otherwise it visits each column with its width and borrowed graphemes, reusing one scratch vector across the row.
• observed — Modified behavior in crates/ghostty-vt/src/lib.rs: Adds comments distinguishing owned grapheme buffers, which retain only their grapheme data, from reusable row scratch buffers.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 3 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 and concisely describes the main change: eliminating per-cell allocations during terminal text reads.
Description check	✅ Passed	The description directly explains the allocation problem, the row-buffer reuse fix, verification results, and test coverage.
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

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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] Optimizes terminal text reading to reduce allocations.

The PR appears safe to merge; no actionable issue remains from the changes reviewed.

Summary

The PR reuses one grapheme buffer per screen row for text reads while retaining tight allocations for callers that need owned cells.

• Adds equivalence and allocation-scaling tests for borrowed row reads.
• Restores the owned-cell allocation behavior at the final head.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Grid[Terminal grid] --> Borrowed[Borrowed row read]
  Grid --> Owned[Owned cell read]
  Borrowed --> Scratch[Reusable grapheme buffer]
  Scratch --> Text[Pane text]
  Owned --> Cell[Owned grapheme vectors]

Loading

_{Reviews (3) · Last reviewed commit: "fix: preserve tight allocations for owne..."}

[akbash-bot]

Regarding the docstring-coverage warning, including the updated 30% result on 7a5574a: the public visitor documents its borrowed-slice lifetime and out-of-range behavior, the test allocator module documents its counting scope, and the owned helper now explains its distinct allocation policy. The remaining private helpers and tests follow existing conventions. No missing API contract was identified, so I am not adding repetitive comments solely to meet the percentage.

[JJLiebig]

@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.

[akbash-bot]

@coderabbitai review
@greptileai

Please review final head 7a5574a, which restores tight allocations for owned grapheme reads while preserving row-buffer reuse.

[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.

[JJLiebig]

Looks good to me now. The owned reads keep their original allocation size and the row reader still reuses the buffer. Akbash MVP as usual.