[JJLiebig]

Client shell snapshots built full public pane responses for every pane and again for agents, reading terminal scrollbars these projections discard. Share pane metadata extraction and add scroll enrichment for public pane/session responses. Retain rendered pane-surface scroll data and existing snapshot contents, focus, IDs, and completion sequences.

Performance

Construction-only microbenchmark, in microseconds: fixed 120x40 terminal geometry, 1/15/118 populated agent panes, 1/2 client encoding fanout, 5 warmups and 40 measured samples per case. Each value below is the median of two run medians from saved parent/candidate executables run in baseline/candidate/candidate/baseline order. The existing just bench-render-scale recipe reaches the actual snapshot_with_completions path; both versions passed it.

Layout / output	Panes	Encoded clients	Before us	After us	Time change
Active, quiet	1	1	6	6.5	+8.3%
Active, quiet	1	2	6	5.5	-8.3%
Active, quiet	15	1	52	49	-5.8%
Active, quiet	15	2	54	48	-11.1%
Active, quiet	118	1	456	388.5	-14.8%
Active, quiet	118	2	473.5	434	-8.3%
Background, quiet	1	1	6	6	+0.0%
Background, synthetic hidden output	1	1	6	6	+0.0%
Background, quiet	1	2	6	6	+0.0%
Background, synthetic hidden output	1	2	6	5.5	-8.3%
Background, quiet	15	1	70.5	71	+0.7%
Background, synthetic hidden output	15	1	114.5	120	+4.8%
Background, quiet	15	2	79.5	68.5	-13.8%
Background, synthetic hidden output	15	2	116.5	117	+0.4%
Background, quiet	118	1	643	575	-10.6%
Background, synthetic hidden output	118	1	938.5	832.5	-11.3%
Background, quiet	118	2	653.5	591	-9.6%
Background, synthetic hidden output	118	2	943	872.5	-7.5%

Negative time changes mean less construction time. One-pane values are near the timer's microsecond rounding; 15-pane results are neutral/mixed, including no measured hidden-output gain. The background cases have one active workspace plus N-1 hidden workspaces; N=1 is a no-hidden-pane control. Hidden bytes are injected sequentially into those hidden workspaces before the timer starts. Client counts cover encoding fanout after the construction timer stops. These controlled in-process results do not establish sustained-writer contention, live-client behavior, an incident fix, or a production CPU reduction.

For N agent panes, the previous shell path performed 2N discarded scrollbar reads: 2/30/236 reads at 1/15/118 panes. The new path performs zero. The regression test counts actual terminal scroll reads, checks public session scroll values, and checks metadata/shell/completion output parity.

Validation

• Focused existing public session, pane-scroll, and terminal-title coverage passed against the candidate release test executable.
• Baseline and candidate just bench-render-scale passed (6 matching manual profiles each); paired saved-executable measurements also passed.
• Zero-scroll-read regression passed; full just check passed (formatting, all-target clippy, 3336 Rust tests with 11 configured skips, maintenance tests, and build). The first full run timed out on a 100 ms oversized-paste notification expectation; focused retry and the full retry passed. The cause remains unproven.
• Installed Review Suite --mode fast: clean dual signoff, rvw_bc760932 complete. GitHub CI passed on f36063c (Linux/macOS/Windows, ConPTY package, contribution and conventional-commit checks). Greptile approved that head (5/5); CodeRabbit completed that head with no actionable comments. Its non-blocking docstring-coverage warning was reviewed and declined in a documented PR disposition.

[kangal-bot]

@coderabbitai review
@greptileai

[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: 4436c45a-f513-4d97-8c72-7b077a48a840

📥 Commits

Reviewing files that changed from the base of the PR and between 746156b and ebe36b5.

📒 Files selected for processing (8)

• src/app/agents.rs
• src/app/api/session.rs
• src/app/creation.rs
• src/pane.rs
• src/pane/terminal.rs
• src/server/client_shell.rs
• src/server/render_scale_benchmark.rs
• src/terminal/runtime.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

Pane metadata lookup now omits scroll metrics, while pane information continues to include them. Session snapshots gather panes from tab layouts, and client-shell snapshots use session metadata. The snapshot encoding benchmark reports construction and total timing statistics and can write output to hidden workspaces.

Suggested reviewers: ogulcancelik

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to ebe36

The snapshot change preserves pane contents while avoiding unused scroll-metric reads in client-shell snapshots. No identified issue prevents merging after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ebe36

The inspected changes preserve public response behavior and existing access scope while avoiding unnecessary reads. No new security issue was established, but review coverage is limited.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected exposure is the application's existing session-wide projection, including active and background workspaces and their pane metadata. The inspected base/head comparison does not establish expanded workspace visibility, a new remotely reachable operation, or additional privileges.

Trust Boundaries and Controls

• observed — The selector is a private builder's function parameter supplied by internal methods, not an attacker-selected callback. Both choices use the same pane-state and public-ID resolution. This supports unchanged projection boundaries, but is not evidence of a complete authentication or tenant-isolation audit.

Resilience and Maintainability Implications

• observed — The added scroll-read counter and its increment are guarded by test configuration. Production scroll enrichment still locks the terminal core and returns absent metrics if the lock or scrollbar query fails; the metadata-only path skips that enrichment without changing pane ownership or committing production state.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 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 and concisely describes the main performance change: avoiding unused terminal reads during client snapshot construction.
Description check	✅ Passed	The description directly explains the snapshot changes, performance results, regression coverage, and validation for 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

• 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.}

[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: 4/5

[Medium risk] Refactors snapshot collection to avoid redundant terminal reads.

The PR does not appear safe to merge while Windows pane shutdown can wait indefinitely for a blocked PTY writer.

Summary

The PR avoids terminal scroll reads when constructing client-shell snapshots while retaining scroll enrichment for public session responses. Changes since the previous review also alter Windows PTY input forwarding and shutdown sequencing.

• Shares pane metadata extraction between snapshot paths and adds a scroll-read regression test.
• Expands snapshot construction benchmarking.
• The Windows shutdown change introduces an unbounded wait when a queued Enter cannot flush.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Pane metadata] --> B{Snapshot consumer}
  B -->|Public session| C[Enrich with scroll metrics]
  B -->|Client shell| D[Project metadata without scroll reads]

Loading

_{Reviews (4) · Last reviewed commit: "Merge branch 'master' into perf/client-s..."}

[JJLiebig]

CodeRabbit's docstring-coverage warning was reviewed. The touched methods are internal Rust metadata collectors, existing benchmark/test helpers, and test-only counters. The change does not add a public API or alter the wire contract; the existing owning regression test proves scroll preservation and projection parity. Expanding comments to satisfy a percentage threshold would add unrelated boilerplate, so the warning is left as a documented non-blocking disposition. Both bots completed on f36063c with no actionable code findings.

[JJLiebig]

@coderabbitai review
@greptileai

[coderabbitai]

⚠️ Action not completed

No files to review.

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]

@coderabbitai
@greptileai

full review after rebase

[coderabbitai]

@JJLiebig I’ll trigger a full review of all PR changes after the rebase, including changes reviewed before.

✅ Action performed

Full review finished.

[JJLiebig]

Note on the Greptile find: Confirmed this and sending a follow up.