Skip to content

fix: hidden pane memory and new pane spawn size - #4873

Merged
ogulcancelik merged 2 commits into
masterfrom
fix/hidden-pane-spawn-memory
Oct 2, 2026
Merged

ogulcancelik merged 2 commits into
masterfrom
fix/hidden-pane-spawn-memory

Conversation

@ogulcancelik

Copy link
Copy Markdown
Collaborator

Hidden panes cost much more memory than visible ones, and new panes start at the wrong size.

Memory. Each pane built a full screen copy at creation just to read its starting colors. Hidden panes never refresh it, so it stays in memory. With 100 panes, a hidden workspace went from 27 to 101 MiB, while the same panes visible used 47 MiB. The starting colors of a fresh terminal are always the same defaults, so we now read them without building the copy. 100 hidden panes now cost 44 MiB, the same as visible.

Size. New panes borrowed the size of whatever pane was first on screen. In a hidden workspace or a headless server, that could be a 1-row pane (#4419), or the whole window. Each pane now starts at the size its own tab layout will give it: full size for a new tab or workspace, its share for a split, the zoomed size for overlays, and the final size of every pane for layout apply. On a live 100-pane run, 99 of 99 split panes started at their final size. On master, all 99 were resized right after creation. Headless #4419: a new workspace next to a tiny first pane now starts at 40x119, where it used to start at 2x120.

Tests cover:

  • render-copy deferral and starting colors
  • first draw after a hidden shrink
  • split-geometry prediction
  • hidden workspace and split spawn size matching the resize step
  • nested layout apply

The new spawn-size and layout tests fail on the old code.

refs #4419

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: 7cb43d4a-9426-4c47-a28f-b15ec8eec72d

📥 Commits

Reviewing files that changed from the base of the PR and between aa811e1 and 7daad6c.

📒 Files selected for processing (1)
  • src/app/api/plugins/mod.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 dimensions now come from the requested placement or final tile layout across workspace, tab, split, plugin, and command creation paths. Layout application passes ordered sizes through recursive pane creation. Pane resize uses shared minimum dimensions. Terminal render-state initialization is deferred until rendering, with tests for panes resized before their first draw.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 7daad

No actionable regression is established by the supplied review context; the described change is suitable to merge subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to aa811

The change affects pane startup sizing and deferred screen preparation. No new access or privilege expansion was established, and existing validation and failure cleanup remain in the reviewed paths. Incomplete verification of interrupted launches and concurrent requests limits confidence.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The established propagation scope is the application's pane, tab, workspace, and terminal-runtime state across API, plugin, command, and respawn paths. The supplied evidence does not establish an expansion into additional services, tenants, credentials, or infrastructure authority.

Trust Boundaries and Controls

  • observed — Layout application resolves the requested tab or workspace, rejects conflicting selectors, and validates the entire layout before creation. The available sizing diff leaves these existing target and launch-validation controls in place; geometry prediction does not substitute for them.

Resilience and Maintainability Implications

  • observed — A recursive layout-construction error invokes rollback of the newly created tab, removes plugin and unattached-terminal records, and requests detached-runtime shutdown. The replacement target is closed only after successful construction. This preserves the inspected partial-failure containment path, without proving cleanup after abrupt host termination.
  • observed — Full rendering holds the terminal-core mutex and returns if snapshot refresh fails. Dirty-patch collection reports fallback on refresh failure, while text and hyperlink readers propagate errors. These controls are counterevidence to consuming an uninitialized deferred snapshot in the inspected readers.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 54 functions across 16 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 identifies both main changes: reduced hidden-pane memory use and corrected new-pane spawn sizes.
Description check ✅ Passed The description directly explains the memory and pane-sizing changes, reported results, affected issue, 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

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

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
✅ 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

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors pane sizing logic for hidden and new panes.

The PR appears safe to merge; no blocking issues were found.

What we checked:

  • Zoomed splits get split sizes: No. split_pane_with_runtime clears zoomed after creating the pane, so the requested split size matches the layout used afterward.
  • First draw refreshes the copy: render updates render_state before reading colors or cells. Removing the update at creation does not make the first draw use the empty copy.

Summary

This PR avoids building a screen copy when a terminal is created. It also starts new panes at sizes calculated from their own layout instead of borrowing the first visible pane’s size.

  • Splits include pane borders, gaps, and the scrollbar gutter.
  • Layout apply calculates each pane’s final size before starting its program.
  • New tests cover hidden panes, nested layouts, starting colors, and drawing after a shrink.
  • No actionable issues were found. Tests were inspected but not run.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Create a pane] --> B[Choose its placement]
  B --> C[Calculate its layout rectangle]
  C --> D[Apply borders, gaps, and scrollbar space]
  D --> E[Apply minimum rows and columns]
  E --> F[Start the terminal at that size]
  F --> G[First draw refreshes the screen copy]
Loading

Reviews (2) · Last reviewed commit: "fix: start new panes at their laid-out s..."

@ogulcancelik
ogulcancelik force-pushed the fix/hidden-pane-spawn-memory branch from aa811e1 to 7daad6c Compare October 2, 2026 21:57
@ogulcancelik

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
✅ 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.

@ogulcancelik
ogulcancelik merged commit 5da0a01 into master Oct 2, 2026
7 checks passed
maciejul pushed a commit to maciejul/herdr-supercharged that referenced this pull request Oct 28, 2026
* fix: stop hidden panes keeping a full-size screen copy from creation

* fix: start new panes at their laid-out size

refs herdrdev#4419
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.

2 participants