Skip to content

Route keyboard/menu equalize_splits through v2ProportionalEqualize so 3+ panes split evenly - #4400

Merged
austinywang merged 4 commits into
manaflow-ai:mainfrom
mvanhorn:fix/4378-cmux-equalize-splits-asymmetric-tree
May 20, 2026
Merged

austinywang merged 4 commits into
manaflow-ai:mainfrom
mvanhorn:fix/4378-cmux-equalize-splits-asymmetric-tree

Conversation

@mvanhorn

@mvanhorn mvanhorn commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  1. Extract the proportional algorithm. In Sources/TerminalController.swift around lines 5141-5160, v2ProportionalEqualize already implements the correct algorithm (recursive leaf-count walk → divider ratio). Lift the core loop into a shared helper (e.g. TerminalController.equalizeSplitsProportionally(in:) or a free function in a new EqualizeSplitsCore.swift) that takes a split tree root and applies the leaf-proportional divider for each node. Both call sites then go through the helper.

  2. Reroute the legacy call site. In Sources/TabManager+EqualizeSplits.swift:47-79, replace the body of equalizeSplits(in:controller:foundSplit:allSucceeded:) so instead of recursing with a hard-coded 0.5, it invokes the new shared helper against the same split tree, threading the existing foundSplit / allSucceeded accounting through. Leave the public signature and call sites untouched so the keyboard binding, SwiftUI menu, and command-palette path all keep working.

  3. Keep v2 path intact. v2ProportionalEqualize continues to be the CLI RPC entry. After the helper extraction, both paths must call the same code. Confirm there is no diverging state (e.g. animation flag, controller routing) by reading the two callers side-by-side.

  4. No schema, no settings change. This is a pure layout-calculation fix; no persisted state, no CLI surface area changes.

Why this matters

cmux 0.64.7 has two equalize-splits code paths that disagree. The keyboard shortcut (default Cmd+Ctrl+=) and SwiftUI menu route through TabManager+EqualizeSplits.swift, which unconditionally calls setDividerPosition(0.5, ...) on every split. The v2 RPC path (cmux rpc workspace.equalize_splits) routes through TerminalController.swift's v2ProportionalEqualize, which counts leaves on each side of every split and sets the divider to leftLeafCount / (leftLeafCount + rightLeafCount).

For any pane count that is not a power of two, the split tree is asymmetric (e.g. 3 panes = A | (B | C)). The 0.5-per-split implementation then ships layout 50/25/25 instead of 33/33/33. The v2 RPC path gives correct 33/33/33. The bug is purely that the keyboard / menu / command-palette paths never adopted the v2 algorithm.

Reporter (branch10480) verified both paths on the same workspace at cmux 0.64.7 and pinpointed the divergent files and line ranges.

Testing

  1. 3 horizontal panes (asymmetric tree): open 3 panes such that tree is A | (B | C), drag dividers to uneven sizes, press Cmd+Ctrl+=. Expected widths: 33/33/33 ± 1%. Today: 50/25/25.
  2. 4 horizontal panes (balanced tree): tree ((A | B) | (C | D)). Expected: 25/25/25/25 (no regression — both algorithms agree on power-of-two trees).
  3. 5 panes (asymmetric): build A | (B | (C | (D | E))) or similar deep right-chained tree, press Cmd+Ctrl+=. Expected: 20/20/20/20/20. Today: deeply skewed.
  4. Mixed horizontal + vertical: A | (B / C) (vertical split inside right side). Expected: A occupies 1/3 width, B/C each occupy 1/3 width and 50% height of right column. Today: A occupies 50% width.
  5. CLI parity: cmux rpc workspace.equalize_splits on the same 3-pane workspace must continue to return {"equalized": true} with the same final layout the keyboard shortcut produces.
  6. SwiftUI menu and command palette ("Equalize Splits") entries must both produce the same layout as the keyboard shortcut.

Fixes #4378

AI was used for assistance.


View in Codesmith
Need help on this PR? Tag @codesmith with what you need.

  • Let Codesmith autofix CI failures and bot reviews

Summary by cubic

Routes the keyboard, menu, and command-palette “Equalize Splits” to the same leaf‑proportional algorithm as v2 RPC so 3+ panes split evenly. Fixes uneven layouts like 50/25/25 by setting dividers based on leaf counts.

  • Bug Fixes
    • Added TerminalController.equalizeSplitsProportionally(...) returning EqualizeSplitsResult; all equalize entry points call it.
    • Removed TabManager’s 0.5-per-split recursion; pass fromExternal: true to setDividerPosition; removed unreachable leaf-count guard.
    • v2 RPC keeps prior equalized response (true if a matching split exists); keyboard/menu still require full success. Orientation filter preserved. 3, 5, and mixed trees now equalize (~33/33/33, etc.); power‑of‑two unchanged. Fixes Keyboard/menu equalize_splits is not truly even for 3+ panes (asymmetric tree: 50:25:25) #4378.

Written for commit d2398b5. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

  • New Features
    • Added proportional divider equalization for split views, automatically adjusting divider positions based on subtree sizes to produce more balanced layouts.

Review Change Stack

@vercel

vercel Bot commented May 19, 2026

Copy link
Copy Markdown

@mvanhorn is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented May 19, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Added EqualizeSplitsResult and a recursive equalizeSplitsProportionally() helper that computes divider positions from subtree leaf counts and updated TabManager to call this helper instead of the legacy fixed-0.5 recursion.

Changes

Proportional split equalization

Layer / File(s) Summary
Proportional equalization helper and result tracking
Sources/TerminalController.swift
EqualizeSplitsResult and equalizeSplitsProportionally() recursively compute each split's target divider as leftLeafCount / (left+right) and apply positions via the controller, returning flags for found splits and overall success.
TabManager delegates to helper
Sources/TabManager.swift
equalizeSplits(tabId:) now calls TerminalController.equalizeSplitsProportionally(...) and returns a boolean derived from the returned foundSplit and allSucceeded flags; the old in-file recursion was removed.

Sequence Diagram(s)

sequenceDiagram
  participant TabManager
  participant TerminalController
  participant BonsplitController
  participant ExternalTreeNode
  TabManager->>TerminalController: equalizeSplitsProportionally(tabId, controller, fromExternal: true)
  TerminalController->>ExternalTreeNode: traverse node (split/pane)
  alt node is split
    TerminalController->>BonsplitController: setDividerPosition(ratio, forSplit: uuid, fromExternal: true)
    BonsplitController-->>TerminalController: Bool success
  end
  TerminalController-->>TabManager: EqualizeSplitsResult(foundSplit, allSucceeded)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through nodes both left and right,
Counting leaves until the ratios are right.
No more halves for an uneven tree,
Each pane gets fair share—hooray for me!

🚥 Pre-merge checks | ✅ 16 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 accurately describes the main change: routing keyboard/menu equalize_splits through the proportional algorithm so 3+ panes split evenly.
Linked Issues check ✅ Passed The code changes directly address #4378 by extracting the proportional algorithm from v2ProportionalEqualize and routing the keyboard/menu/command-palette paths through it, enabling true even splits for 3+ panes.
Out of Scope Changes check ✅ Passed All changes are in-scope: the helper extraction from TerminalController.swift, TabManager.swift refactoring to delegate to the helper, and no unrelated modifications to schema or settings.
Cmux Swift Actor Isolation ✅ Passed New static method on @MainActor TerminalController maintains proper actor isolation. EqualizeSplitsResult contains only Bool (inherently Sendable), and all call sites execute in @MainActor context.
Cmux Swift Blocking Runtime ✅ Passed New code adds EqualizeSplitsResult struct and equalizeSplitsProportionally() recursive tree traversal helper—no blocking synchronization patterns (semaphores, Task.sleep, locks) introduced.
Cmux No Hacky Sleeps ✅ Passed PR modifies only Swift files (TerminalController.swift, TabManager.swift); check explicitly covers non-Swift code only. Swift sleeps handled by separate rule.
Cmux Swift Concurrency ✅ Passed New code adds synchronous tree traversal with no legacy async patterns: no DispatchQueue, async/await, Task creation, @escaping closures, or fire-and-forget Tasks.
Cmux Swift @Concurrent ✅ Passed No async functions in PR changes. All new/modified code (EqualizeSplitsResult struct, equalizeSplitsProportionally, equalizeSplits) is synchronous, so @concurrent annotation rules do not apply.
Cmux Swift File And Package Boundaries ✅ Passed Adds 43 lines to existing oversized TerminalController file (+0.28%); focused bug fix extracting tree traversal algorithm; fits existing split management patterns; preserves clear extraction path.
Cmux Swift Logging ✅ Passed No logging statements (print, debugPrint, dump, NSLog, Logger) detected in new code in TerminalController.swift or modified code in TabManager.swift.
Cmux User-Facing Error Privacy ✅ Passed PR is algorithmic refactoring with zero string literals in new code, zero sensitive data patterns, and no user-facing error messages added.
Cmux Full Internationalization ✅ Passed PR contains only internal code refactoring with no user-facing Swift text, Info.plist changes, string catalog modifications, or web UI updates that would require localization.
Cmux Swiftui State Layout ✅ Passed Changes refactor split equalization logic without introducing new SwiftUI state. No new @Published properties, @Observable declarations, GeometryReader measurement, or render-time mutations added.
Cmux Architecture Rethink ✅ Passed Correctness fix consolidating proportional equalize algorithm with clear ownership, defined invariants, and no anti-patterns: no sleeps, dispatch delays, polling, locks, observers, or side-channels.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR adds internal split equalization helpers and refactors TabManager to use them; no new NSWindow, NSPanel, NSWindowController, SwiftUI Window, or WindowGroup objects created.
Description check ✅ Passed The PR description is comprehensive and detailed, covering the summary, rationale, testing scenarios, and implementation approach.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@greptile-apps

greptile-apps Bot commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates two divergent "equalize splits" code paths — the keyboard/menu path (which always set every divider to 0.5) and the v2 RPC path (which used leaf-proportional positioning) — into a single shared TerminalController.equalizeSplitsProportionally static helper. The fix corrects the long-standing bug where 3+ asymmetric panes produced 50/25/25 instead of 33/33/33.

  • Algorithm unification: The old TabManager.equalizeSplits(in:controller:foundSplit:allSucceeded:) recursion and its hardcoded 0.5 are removed; both call sites now go through the new static helper with the proportional leaf-count algorithm.
  • EqualizeSplitsResult struct promoted: Previously private to TabManager, it is now a TerminalController-level type accessible to both callers, carrying foundSplit, allSucceeded, and the derived didFullyEqualize.
  • v2 RPC response change: "equalized" now reflects equalizeResult.foundSplit rather than the old private success bool from v2ProportionalEqualize, a slight semantic shift noted in the inline comment.

Confidence Score: 5/5

Safe to merge; the core algorithm change is correct and both call sites are properly unified through the shared helper.

The leaf-proportional algorithm is mathematically correct for all tree shapes. Both call sites now go through the same static helper, eliminating the divergence that caused the 50/25/25 bug. The only finding is a minor ordering issue where foundSplit is set before UUID validation succeeds, which could cause the v2 RPC to return "equalized": true when nothing was actually moved — but since split IDs are system-generated UUIDs this path is unreachable in practice.

Sources/TerminalController.swift — specifically the ordering of foundSplit = true relative to the UUID guard inside equalizeSplitsProportionally.

Important Files Changed

Filename Overview
Sources/TerminalController.swift Adds equalizeSplitsProportionally static helper and EqualizeSplitsResult struct; v2WorkspaceEqualizeSplits now delegates here. Subtle semantic change: "equalized" in the RPC response now uses foundSplit (true even on UUID parse failure) rather than the old guard-gated bool.
Sources/TabManager+EqualizeSplits.swift Old recursive 0.5-per-split implementation removed; equalizeSplitsOnce now delegates to TerminalController.equalizeSplitsProportionally. Logic for foundSplit/allSucceeded tracking is preserved through the shared result type.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Keyboard Cmd+Ctrl+=] --> B[TabManager.equalizeSplits]
    C[SwiftUI Menu / Command Palette] --> B
    D[CLI: cmux rpc workspace.equalize_splits] --> E[v2WorkspaceEqualizeSplits]

    B --> F[equalizeSplitsOnce\nfromExternal: true]
    E --> G[equalizeSplitsProportionally\nfromExternal: true\norientationFilter: optional]

    F --> H[TerminalController.equalizeSplitsProportionally static]
    G --> H

    H --> I{for each split node depth-first}
    I --> J[count leaves left/right - position = leftLeaves / total]
    J --> K[setDividerPosition]
    K --> L[EqualizeSplitsResult: foundSplit / allSucceeded]

    L --> M{caller}
    M -->|TabManager checks didFullyEqualize| N[scheduleFollowUp if foundSplit]
    M -->|v2 RPC| O[equalized: foundSplit]
Loading

Reviews (5): Last reviewed commit: "Clean equalize split leaf-count guard" | Re-trigger Greptile

Comment thread Sources/TerminalController.swift Outdated
Comment thread Sources/TerminalController.swift Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@mvanhorn
mvanhorn force-pushed the fix/4378-cmux-equalize-splits-asymmetric-tree branch from 98d0740 to 1de4cd7 Compare May 20, 2026 00:16
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Good catch from Greptile -- the original push added the proportional helper but never wired it into the call site. Fixed in 1de4cd7: TabManager.equalizeSplits(tabId:) now calls TerminalController.equalizeSplitsProportionally directly, and the dead recursive helper with the hardcoded 0.5 divider position is gone. The keyboard, menu, and command-palette paths all flow through the proportional helper now.

@vercel

vercel Bot commented May 20, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment May 20, 2026 10:14am

Resolved conflicts by keeping main's pane-resize extraction in TabManager.swift/TerminalControllerPaneResizeSupport.swift and porting the PR's equalize reroute into the new TabManager+EqualizeSplits.swift shape.

Verification:

- Verified TabManager+EqualizeSplits.swift now calls TerminalController.equalizeSplitsProportionally for the keyboard/menu/command-palette path.

- Verified v2WorkspaceEqualizeSplits calls the same helper with fromExternal: true and preserves the orientationFilter parameter.

- Verified no setDividerPosition(0.5, ...) call remains in the equalize path and no conflict markers remain.

- Identified equalize-related tests in cmuxTests/ and tests/; local test/build execution was skipped per repo policy and the explicit no-xcodebuild/no-reload constraint.
Keep v2WorkspaceEqualizeSplits on the shared proportional helper while returning true when a matching split was found, matching the prior v2ProportionalEqualize response contract. The keyboard/menu path still uses didFullyEqualize so failed divider updates remain visible there.

Verification:

- Verified both v2 and TabManager equalize paths still call TerminalController.equalizeSplitsProportionally.

- Verified no setDividerPosition(0.5, ...) or duplicate v2ProportionalEqualize/v2CountLeaves implementation remains.

- Ran git diff --check only; no local build/test/reload per constraints.
Addresses low-priority Greptile feedback by removing the unreachable totalLeafCount > 0 check in the proportional equalize helper.

Verification:
- git diff --check passed.
- Both TabManager+EqualizeSplits and v2WorkspaceEqualizeSplits still call TerminalController.equalizeSplitsProportionally.
- No setDividerPosition(0.5, ...) calls remain in the equalize path.
@austinywang
austinywang merged commit 542130c into manaflow-ai:main May 20, 2026
18 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request May 20, 2026
6 tasks done
@austinywang austinywang mentioned this pull request May 22, 2026
@mvanhorn

Copy link
Copy Markdown
Contributor Author

@austinywang appreciate the merge. Lifting the recursive leaf-count walk into the shared helper makes equalize_splits behave consistently for 3+ panes instead of falling back to the older path.

This branch was successfully deployed

1 active deployment
Preview – cmux — d2398b53 Deployed May 20, 2026 by vercel[bot]
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.

Keyboard/menu equalize_splits is not truly even for 3+ panes (asymmetric tree: 50:25:25)

2 participants