Investigate split window blank (#3386)

[austinywang]

Closes #3386

Summary

• Centralizes terminal split creation and placeholder repair through shared lifecycle helpers in Workspace so every new terminal split publishes the split event and immediately schedules the event-driven layout follow-up with geometry and focus.
• Keeps surface.split_off from emptying a source pane by preserving the single-tab guard and returning invalid_state for that API case.
• Adds split-render regression coverage for both v1 and v2 socket tests, plus Swift unit coverage for the surface.split_off source-pane invariant.

Testing

• Not run locally per repo policy and user instruction.
• CI covers the Swift unit regression in cmuxTests/TerminalControllerSurfaceSplitOffTests.swift.
• CI covers the split-render regression through tests/test_split_flash_and_layout.py and tests_v2/test_split_flash_and_layout.py, using tests/split_render_helpers.py to verify the newly created split panel visibly renders deterministic terminal output.

Notes

• This branch was merged with current main before the final CI pass so required checks evaluate the up-to-date integration state.
• Demo video omitted; the regression is covered by the CI-rendered split panel assertions.

---

Note

Medium Risk
Touches terminal split/placeholder-repair lifecycle and layout follow-up scheduling, which can impact focus and geometry timing during splits; regressions would show up as blank/incorrectly sized panes.

Overview
Centralizes terminal split creation/repair through new Workspace helpers (beginTerminalSplitPaneLifecycle, beginTerminalSplitSurfaceLifecycle, commitNewTerminalSplitPane) so every new terminal pane/surface consistently publishes cmux events and immediately triggers event-driven layout follow-up with geometry (and optional focus).

Updates placeholder-repair and UI/programmatic split paths to use the shared lifecycle and ensures layout follow-up runs after creating replacement terminals (reducing split “blank/flash” states).

Adds regression coverage: a Swift unit test ensuring surface.split_off rejects splitting the only tab in a pane (invalid_state), plus shared Python helpers and enhanced v1/v2 split layout tests that assert newly split terminals actually render output.

^{Reviewed by Cursor Bugbot for commit bf2fb95. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Tests

  • Added test helpers for validating terminal split rendering output.
  • Added test coverage for surface.split_off API validation on terminal panes.

• Refactor

  • Consolidated terminal split lifecycle management with centralized helper methods.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 19, 2026 6:49am
cmux-staging	Building	Preview, Comment	May 19, 2026 6:49am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 64c7e456-4a60-4cf2-9bf5-172c5e02e366

📥 Commits

Reviewing files that changed from the base of the PR and between 1faf4e6 and bf2fb95.

📒 Files selected for processing (3)

• Sources/Workspace.swift
• cmux.xcodeproj/project.pbxproj
• cmuxTests/TerminalControllerSurfaceSplitOffTests.swift

---

📝 Walkthrough

Walkthrough

Workspace centralizes terminal split/surface lifecycle into private helpers and updates call sites to use them. New Python test helpers assert split terminals render output; split-creation tests now invoke those helpers. A new Swift unit test is added and wired into the cmuxTests target.

Changes

Terminal Split Lifecycle Refactoring and Rendering Verification

Layer / File(s)	Summary
Terminal split lifecycle helpers and refactoring Sources/Workspace.swift	Introduces beginTerminalSplitPaneLifecycle(...) and beginTerminalSplitSurfaceLifecycle(...) to standardize model event publication and start event-driven layout follow-up, and commitNewTerminalSplitPane(...) to commit bonsplit pane creation and optional divider positioning. Updates newTerminalSplit(...), splitPaneWithNewTerminal(...), and BonsplitDelegate didSplitPane paths to use these helpers.
Split rendering test assertion helpers tests/split_render_helpers.py	Adds _panel_snapshot_retry with targeted retry logic, _snapshot_ratio to compute a normalized changed-pixel ratio, and assert_split_terminal_renders_output(c, panel_id) which resets a panel, sends deterministic output, polls snapshots until rendering output change exceeds a dynamic threshold, and raises cmuxError with diagnostics on failure.
Integration of render assertions into split-creation tests tests/test_split_flash_and_layout.py, tests_v2/test_split_flash_and_layout.py	Adjusts sys.path insertions and imports the new helper. Both tests capture the panel id returned by c.new_split("right") and call assert_split_terminal_renders_output to verify the newly created split renders expected output, in addition to existing flash and layout health checks.
Xcode project wiring and Swift unit test cmux.xcodeproj/project.pbxproj, cmuxTests/TerminalControllerSurfaceSplitOffTests.swift	Adds TerminalControllerSurfaceSplitOffTests.swift to the test target and project file. Adds TerminalControllerSurfaceSplitOffTests XCTest class with testSurfaceSplitOffRejectsOnlyTabSourcePane and helpers to construct an NSWindow and dispatch v2 JSON envelope requests to TerminalController.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I nibbled at the split, then hopped to see,
New lifecycle paths now tidy as can be,
Tests snap frames until the output shows,
A rendered pane blooms where the seed was sown,
Hooray — the rabbit dances, tail all aglow.

🚥 Pre-merge checks | ✅ 16 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (16 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Investigate split window blank (#3386)' directly references the issue being fixed and describes the main problem addressed in the changeset.
Description check	✅ Passed	The description provides a comprehensive summary of changes, testing approach, and includes notes about CI coverage and branch integration.
Linked Issues check	✅ Passed	The PR addresses issue #3386 by centralizing terminal split lifecycle helpers, implementing layout follow-up scheduling, and adding regression test coverage via Swift unit tests and Python socket tests.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing the split window blank issue: lifecycle helpers in Workspace, surface.split_off guard preservation, test coverage for splits, and related project configuration.
Cmux Swift Actor Isolation	✅ Passed	No actor isolation violations. New private methods in @MainActor Workspace class use only value/safe types. Test file properly marked @MainActor.
Cmux Swift Blocking Runtime	✅ Passed	PR introduces no new blocking patterns in production Swift code. New helpers refactor existing async mechanisms (asyncAfter). Tests properly use time.sleep for instrumentation.
Cmux No Hacky Sleeps	✅ Passed	All sleep() calls are in test files (tests/, tests_v2/) and test-only scaffolding. No production Python/JS/TS/shell code changes. Rule allows deterministic test sleeps.
Cmux Swift Concurrency	✅ Passed	New split lifecycle helpers contain no problematic async patterns. DispatchQueue.main.asyncAfter usage is with stored workitem and version tracking for UI isolation—allowed.
Cmux Swift @Concurrent	✅ Passed	Swift changes comply with concurrency rules. New synchronous helper methods properly inherit @MainActor isolation. No invalid @concurrent, async/nonisolated, or actor isolation violations found.
Cmux Swift File And Package Boundaries	✅ Passed	Adds 125 lines to oversized Workspace.swift, below 250-line threshold. Focused bug fix for #3386 with only private helpers. No public API expansion or new responsibilities.
Cmux Swift Logging	✅ Passed	No Swift logging violations found. Production code adds no print, debugPrint, dump, NSLog, or file I/O. New helpers follow proper encapsulation. Tests use XCTest assertions. No secrets exposed.
Cmux User-Facing Error Privacy	✅ Passed	PR does not violate user-facing error privacy rules. All error messages are pre-existing, generic, and safe. No sensitive data exposed.
Cmux Full Internationalization	✅ Passed	No user-facing text violations detected. Production changes only add internal event identifiers (origin, layoutReason) for event payloads/logging, not UI text. Test files are exempt.
Cmux Swiftui State Layout	✅ Passed	No new SwiftUI state violations. Refactors private helpers in existing Workspace class and adds test files with no SwiftUI state patterns.
Cmux Architecture Rethink	✅ Passed	Consolidates split lifecycle into three helpers eliminating duplication. Clarifies ownership and routes all splits consistently. No new timing patches or architectural debt added.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	New test file creates only test-only NSWindow fixtures with proper cmux.* identifiers. No user-visible windows added. Workspace.swift refactors split logic without new windows.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-3386-split-window-blank

---

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 and usage tips.}

[greptile-apps]

Greptile Summary

This PR centralizes the terminal split lifecycle into three shared Workspace helpers (commitNewTerminalSplitPane, beginTerminalSplitPaneLifecycle, beginTerminalSplitSurfaceLifecycle) and wires beginEventDrivenLayoutFollowUp into the paths that were previously missing it, fixing the blank-pane regression on programmatic split, drag-to-split placeholder repair, and UI-split paths.

• Adds commitNewTerminalSplitPane to consolidate bonsplitController.splitPane, divider-position seeding, model-event publication, and geometry-inclusive layout follow-up into a single reusable path, used by both newTerminalSplit and splitPaneWithNewTerminal.
• Adds beginTerminalSplitSurfaceLifecycle and surfaces it through the placeholder-repair and fallback paths in the BonsplitDelegate.didSplit callback, while removing the now-redundant ad-hoc scheduleTerminalGeometryReconcile() call from the UI-split async block.
• Adds Swift unit coverage (TerminalControllerSurfaceSplitOffTests) for the restored single-tab source-pane guard in surface.split_off, and Python render-assertion coverage (v1 + v2) to verify newly created split panes accept input and visibly render output.

Confidence Score: 5/5

Safe to merge — all three split paths now call beginEventDrivenLayoutFollowUp, the single-tab source-pane guard is restored, and the removed scheduleTerminalGeometryReconcile is fully subsumed by the new lifecycle helpers.

The production Swift changes are functionally correct: the version-invalidation mechanism in beginEventDrivenLayoutFollowUp safely handles the double-call that occurs in the newTerminalSplit focus path, and the tab is already selected by createTab before the geometry pass runs in the UI-split path. No bad state is left representable. Test coverage targets the specific invariant that was previously unguarded.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/Workspace.swift	Adds three lifecycle helpers that consolidate split-pane creation and layout follow-up; wires beginEventDrivenLayoutFollowUp into paths that were previously missing it; removes redundant scheduleTerminalGeometryReconcile() from the UI-split async block. Logic is sound: the version-invalidation mechanism safely handles the double-call in the suppressReparentFocusUntilLayoutFollowUp path.
cmuxTests/TerminalControllerSurfaceSplitOffTests.swift	New Swift unit test verifying surface.split_off returns invalid_state when the source pane has only one tab; correctly uses @testable import, restores shared singleton state via defer, and asserts both the error structure and the post-call pane/tab invariant.
tests/split_render_helpers.py	Shared render-assertion helper extracted into tests/; uses polling with fixed time.sleep intervals. The noise-adaptive threshold is reasonable but can be inflated by cursor-blink on slow CI runners — flagged in a previous thread and not changed here.
tests/test_split_flash_and_layout.py	Extends existing split-flash regression test to capture the new split panel ID, detect the replacement terminal created by drag-to-split placeholder repair, and assert both panels render visible output.
tests_v2/test_split_flash_and_layout.py	Mirrors the v1 test changes identically for the v2 socket path; functionally equivalent to tests/test_split_flash_and_layout.py after the additions.
cmux.xcodeproj/project.pbxproj	Adds TerminalControllerSurfaceSplitOffTests.swift to both the file reference list and the test target Sources build phase; no unrelated changes.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Workspace
    participant commitHelper as commitNewTerminalSplitPane
    participant bonsplit as BonsplitController
    participant lifecycle as beginTerminalSplitPaneLifecycle
    participant followUp as beginEventDrivenLayoutFollowUp

    Note over Caller,followUp: Programmatic split path (newTerminalSplit / splitPaneWithNewTerminal)
    Caller->>Workspace: newTerminalSplit(from:orientation:focus:)
    Workspace->>commitHelper: commitNewTerminalSplitPane(...)
    commitHelper->>bonsplit: splitPane(sourcePaneId:orientation:withTab:)
    bonsplit-->>commitHelper: newPaneId
    commitHelper->>lifecycle: beginTerminalSplitPaneLifecycle(...)
    lifecycle->>lifecycle: publishCmuxSplitCreated
    lifecycle->>followUp: beginEventDrivenLayoutFollowUp(includeGeometry: true)
    followUp->>followUp: scheduleLayoutFollowUpAttempt() [asyncAfter(0)]
    commitHelper-->>Workspace: newPaneId
    Workspace->>Workspace: suppressReparentFocusUntilLayoutFollowUp / focusPanel
    Note over followUp: Layout follow-up runs after sync code completes

    Note over Caller,followUp: UI-split path (BonsplitDelegate.didSplit)
    bonsplit->>Workspace: didSplit(originalPane:newPane:orientation:)
    Workspace->>Workspace: bonsplitController.createTab(inPane: newPane)
    Workspace->>lifecycle: beginTerminalSplitPaneLifecycle(origin:"ui_split")
    lifecycle->>followUp: beginEventDrivenLayoutFollowUp(includeGeometry: true)
    Workspace->>Workspace: DispatchQueue.main.async [selectTab, focusReconcile]

    Note over Caller,followUp: Placeholder repair path (drag-to-split)
    bonsplit->>Workspace: didSplit — !hasRealSurface
    Workspace->>Workspace: reuse placeholder tab → TerminalPanel
    Workspace->>Workspace: beginTerminalSplitSurfaceLifecycle(origin:"placeholder_repair")
    Workspace->>followUp: beginEventDrivenLayoutFollowUp(includeGeometry: true)

Loading

_{Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 34c0932. Configure here.}