Right column: measure panes by their final layout width, so an open right after a split is a tab, not a split below

Merged
LA
6d ago
+127-7

Summary

Right after a split, a link or an agent open beside a pane could land as a new split below the pane on the right. It should be a tab in that pane. This happened in windows narrower than about 960 pt, and only while the split's spring ran, so scripts and agents that split and then open at once hit it most.

Cause: LinkOpener.width(of:) read the pane view's bounds. While the split's spring runs, the source still shows its width from before the split, because its content is held at that size until the spring lands. The new pane starts as a zero-width sliver, which reads as nil, and nil sets no width limit. So RightColumnPlacement.decide saw a wide source and an unmeasured right pane, and picked splitBelow. With both panes measured at their final widths, it picks tab. Found by the independent review of #29787.

Fix: width(of:) now reads LayoutRootView.restingContentWidth(of:). This is the pane's frame in the geometry its screen's springs head for (the baseGeometry the right column is already read from), less the pane padding. The width now comes from one place, so both entry points get the fix:

  • links: LinkOpener.target(from:), which covers openWeb, openFile, openFolder and chat-dock tools through ChatColumnPlacement.openTool
  • agent opens: LinkOpener.rightColumn(beside:), which covers AgentTabStore.rightColumn (AgentSidePanes, AgentSubagentPanes) and AgentBesidePlacement

Action budget: the new read is a dictionary lookup into geometry that is computed when the layout syncs. It adds no layout pass and no wait to a handler. A pane that no screen view lays out (a pane with no workspace, as in SideOpenStripTests) still reads nil, as before.

Owners: LinkOpener and RightColumnPlacement have no OWNERS row, so they go to the Performance Lead. The placement rule is Leo's (#28989, #29684) and is unchanged. LayoutRootView.swift is in the "Layout motion" row (PL-motion-core); the only change there is one new read-only accessor.

Testing

cmux-ci run --class light --display --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=RightColumnAfterSplitTests --env CMUX_TEST_REQUIRE_GUI=1

New test RightColumnAfterSplitTests.anOpenRightAfterASplitReadsThePanesFinalWidths. It uses a same-tree daemon and a 940 pt window. The test drives the layout clock itself (driver.isManual), so the spring stays at its first frame on every host. It splits right, then at once:

  • checks the source's measured width
  • checks the agent path's decision through services.agentTabs.rightColumn
  • opens a link

It expects a tab in the terminal-only right pane and no new split.

  • Red at ade9546b00de (test only, merged with feat-cmux-next), job 2f3bc17c13cb08a78ffb1630 on cmuxs-Mac-mini-4. Three failures:

    • the source read 720 pt (its width before the split)
    • the agent column was Column(sourceWidth: 720, widths: [:]), decided as splitBelow
    • the link target was splitBelow
  • Green at 3af8e0a05961 (head), job a6ce3d5aa1dfab7b603114df on cmux13s-Mac-mini. These suites passed: RightColumnAfterSplitTests, RightColumnPlacementTests, LinkOpenPlacementTests, RightColumnTurnTests, SideOpenStripTests, RightColumnScreenTests, RightColumnEntryPointTests.

  • Display proof at the head 3af8e0a05961: job 0b3b961d1c808367eb9bf7f5 (gui-session-v1, cmux8s-Mac-mini), state done. All 7 suites passed: RightColumnAfterSplitTests, RightColumnPlacementTests, LinkOpenPlacementTests, RightColumnTurnTests, SideOpenStripTests, RightColumnScreenTests, RightColumnEntryPointTests.

Observed / Not run

  • Observed in the same green job: ClickedLinkFocusTests passed 8 of 10 tests. It now passes aLinksTabInAnotherPane... and aChatChipClickedAgain... on cmux13, the host where #29787 saw them fail. At the base they still fail (job 47791f5f3045e15de53ef5b2 at ade9546b, cmux-austin-mini-0). So this race was a cause of those failures.
  • Base findings, not caused by this PR: these tests fail the same way at the base ade9546b (job 47791f5f3045) and at the head:
    • ClickedLinkFocusTests.anAgentsOpenFromALoneChatRefocusesTheChatAsTheApp (:256), the known failure from #29787
    • ClickedLinkFocusTests.theHeadersPreviewBesideThePageTakesTheFocusAndItsCloseGivesItBack (:285)
    • all 4 ChatDockToolPlacementTests (openTool returns false)
  • Not run: the full CmuxNextAppTests target, and a live tagged build.
  • Not run: a hidden or parked workspace. It now reads the geometry from when it last laid out, not nil. I read this from the code and did not test it.

Changelog

  • Fixed: A link or agent panel opened right after a split now opens as a tab in the pane on the right, not as a new split below it, in narrow windows.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes placement of links and agent panes opened right after a split: instead of landing in a new split below the pane on the right, they now land as a tab in that pane, in narrow windows.

LinkOpener.width(of:) read a pane's view bounds, which are stale while a split's spring runs: the source still shows its width from before the split, and the new pane starts as a zero-width sliver (nil). With a wide source and an unmeasured right pane, RightColumnPlacement.decide chose splitBelow. The width now comes from LayoutRootView.restingContentWidth(of:) — the frame the pane rests at in the geometry its screen's springs head for — so links and agent opens both read the final widths. The new read is an O(1) lookup into geometry computed on layout sync, adding no layout pass and no wait.

  • LinkOpener.width(of:) reads restingContentWidth(of:) instead of the pane view's bounds.
  • LayoutRootView.restingContentWidth(of:) returns nil for a pane no screen view lays out, as before.
  • Adds RightColumnAfterSplitTests.anOpenRightAfterASplitReadsThePanesFinalWidths, which drives the layout clock so the split's spring stays at its first frame on every host.