Repository navigation
cmux-next: resize keys move the divider in the arrow's direction in a strip of columns and at a side dock; one press is one undo step - #34123
Conversation
…row's direction; one press is one undo step (red) FRAME OWNER REFACTOR KEYS (lead decision, STATUS 8923). Resize Pane Left/Right on a strip column with no horizontal split grew or shrank the focused column alone: the last column's left edge (the divider a person sees) moved against the arrow, and the columns after a middle one moved with it. A right dock grew with Right, so its inner edge moved left. One press must move one divider one step toward the arrow, and change only the two sides of it. A trade is one press: its two widths go out as one daemon transaction, so Undo Layout Change takes it back in one step (each command took its own number). Red job: pending.
…ow's direction; one press is one undo step FRAME OWNER REFACTOR KEYS (lead decision, STATUS 8923; Lawrence asked the lead to decide). One press moves exactly one visible divider one step toward the arrow, as Ghostty's resize_split does, and changes only the two sides of that divider. - Strip columns (no split along the axis): the divider is the column's right edge when a strip column follows it, else its left edge (the last column). The two columns trade width (`ShownResizeRange.trade`, the Option-drag trade): the columns after them and the camera stay. The press keeps the left column of the pair in place and reveals nothing (`ColumnScrollState.keptColumn`; camera rule L1 kept the focused column, whose left edge is the divider when it is the last one). The trade is whole device pixels and stops at a column's minimum, the widest share, and the edge of the strip the window shows (a press past it would move the divider out of view). One column alone moves its right edge (as before). - Side docks: the inner edge moves with the arrow (a right dock shrinks with Right). - Splits, rows, bands: unchanged. - Undo: every value of one layout transaction goes out under one daemon transaction (`GestureTransactionNumbers`), so the daemon keeps one undo entry for a trade, and for an Option or Command drag's release too (each command took its own number: one press, or one such release, was several undo steps). - `shownResizeRange` and `keyboardResizeWillChange` move to LayoutRootView+KeyResize.swift. - Specs: PaneResize's doc comment, terminal-shortcuts.md (Resize pane), column-scroll.md L6. Red: bec924e7d90dc1e88a29d9f4 at 68b7d12 (KeyboardResizeArrowDirectionTests, KeyboardResizeOneUndoStepTests): FAIL, 8 of 10 tests (a single column's right edge and a left dock already moved with the arrow). Green: the same command at this commit, job id in the PR body.
The first version assumed the strip camera rests at 0 with the middle column focused. In the test window the screen gets its size after the first sync, so the camera reveals the focused column and the next one shows (the divider was mid-window, and the press may move it: green bec924e7's successor b1940035 showed that). Now the strip shows its start (focus on p0), a click focuses p1 (no scroll: it is fully visible), and the test requires the divider between p1 and p2 at the window's edge before it presses: Right is refused, Left moves the divider into the window. Red (the base plus this test, no fix): scratch commit ed71b45, job id in the PR body. Green: the same command at this commit, job id in the PR body.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Passes: CI passes on CI passes on Written by web and size checks: green Shipped in:
|
… outer edge; a dock at its cap or a lone dock refuses the press (red) Review findings on #34123 (Codex FIX at 3f079dc): - ShownResizeRange.trade started a column shown at its minimum from the unrounded minimum, while ColumnStripGeometry.frames shows it at the minimum rounded up to a pixel: with a minimum of 200.1 pt, a press grew it by a pixel less than its neighbor shrank, so the pair's outer edge moved a pixel (scale 1 and 2). - A side dock's press had no shown limit: a dock shown at its share cap (or at the room the strip leaves) took a wider stored width and moved nothing, and the press back took several presses. - A screen whose only column is a dock shows it full width; a press changed its stored width and moved nothing. - aDividerAtTheWindowsEdgeDoesNotMoveOutOfIt checked both sides of the divider against the window's edge, which holds only with a strip gap of at most 1 pt; it checks p1 shown whole and p2's left edge at the window's edge now. Red: job id in the PR body.
…side dock steps inside the widths it shows; a lone dock refuses the press Review findings on #34123 (Codex FIX at 3f079dc): - ShownResizeRange.trade: a column's floor is its minimum rounded up to a pixel (as ColumnStripGeometry.frames widens it), so a press from a column shown at its minimum moves both sides of the divider by the same whole pixels and neither outer edge. - ShownResizeRange.dockWidth: a side dock shows between its panes' minimum and its share of the window, and a docked dock up to the room the strip leaves at its minimum width beside the other docked dock; PaneResize steps the dock inside it, so a press at the cap is refused and the press back is a whole step. Past that room two docked docks share it in proportion (DockStripGeometry.place), so a press there moves both inner edges: the one case where only one divider cannot move. - A screen whose only column is a dock shows it full width: PaneResize refuses the press. Red: the commit before this one (job id in the PR body). Green: job id in the PR body.
…r before and after the f32 echo; a trade settles at its last reply; a refused press keeps no camera anchor (red) Claude review of #34123 at 3f079dc (FIX): - On a strip of an odd number of device pixels 0.5 is an exact half pixel, and the column whose ideal end reaches the window's edge is cut to it: a trade of whole pixels on each stored width rounded the pair two ways or lost the cut, so the last column's right edge moved a pixel at the press (1201 pt at 1x: 1802 -> 1803) and again at the daemon's f32 echo (-> 1801). New: everyPressOnAnOddPixelStripMovesOnlyTheDivider (1x and 2x, camera at the strip's start and end, each press checked at the press and after the echo). - The first of a trade's two replies settled the whole transaction, so a daemon snapshot before the second width's event dropped that width's override: for a frame the right column showed its old width. New: aTradeSettlesWhenItsLastWidthIsAnswered. - A press whose widths the app refused at once never synced, so its kept camera column stayed for the next sync, which skipped its reveal. New: aRefusedPressLeavesTheNextRevealAlone. - The undo test reads GestureTransactionNumbers.latest (the App accessor goes). Red: job id in the PR body.
…s placement; a trade settles at its last reply; a press's camera anchor ends with its sync Claude review of #34123 at 3f079dc (FIX): - ShownResizeRange.trade moves whole pixels from the widths the columns show, and picks the new stored widths so the pair's ideal sum stays (the columns after it and the cut of a column that ends at the window's edge do not change) and both ideal widths sit the same way from their pixels, away from a rounding tie where the sum allows; else the shown widths themselves. The widths are what the daemon keeps (f32), so its echo moves nothing. It places the candidate as the screen does and takes it only when exactly the divider moves by the step; else the press is refused. - LayoutModel counts the ended values of each transaction (TransactionReplies) and settles it at the reply to its last one, so a snapshot between a trade's two events (the store applies events once per display frame) no longer drops the second width's override. The same held for Option and Command drag releases and a split's sizing. GestureOverride (the override value type) moves out of the LayoutModel type to keep it under its count. - PaneResize.press ends the press's kept camera column after its own sync (LayoutRootView.keyboardResizeDidChange), so a press whose widths were refused at once leaves the next reveal alone. - The App's nextGestureTransaction accessor goes (tests read GestureTransactionNumbers); its doc says a dock pin with a width is still two undo steps. Highlight wording. Red: the commit before this one (job id in the PR body). Green: job id in the PR body.
|
Found 141 test failures on Blacksmith runners: Failures
...and 131 more test failures. View all on Blacksmith: run 41725579375, run 41725579322, run 41725579645
|
…wner-keys-direction
…s neither, an early reply settles nothing, a stand-in column refuses the press (red) Codex re-review of #34123 at 7dc25a2 (FIX): - The app refused a trade's first width at once (no handle for its pane), which rolled the press back, and the press then set and sent the second width anyway. - A reply in the turn of the first width's command settled the trade before its second width was counted, so a snapshot before the second event showed the first column at its old width. - A trade beside a stand-in column (a pane the daemon has not made) sent both widths; the daemon applies one and refuses the other: half a trade. - The odd-pixel walks now require every press to move the whole step. Red: job id in the PR body.
…nge, all or nothing; a trade beside a stand-in column is refused at once Codex re-review of #34123 at 7dc25a2 (FIX): - EndedValues (CmuxNextLayout): the values a key press or a drag's release ends with are checked before anything shows (every target exists and LayoutModel.sendsNow can send it now), shown in one turn, counted before the first goes out (the transaction settles at the reply to its last value), and sent under one transaction; a value the app refuses while they go out stops the rest, and the transaction shows the daemon's values again. - LayoutHandleMap.sendsNow (CmuxNextBridge): a split needs a handle, a column's pane a handle of a pane the daemon made (ProvisionalPane.isStandIn: a stand-in's commands are refused, PaneNotReady). The app wires it next to its intent handler. A key trade beside a column the daemon has not made is refused at once, before anything moves (as other pane verbs on a stand-in), instead of half a trade at the daemon. - PaneResize.press sends split ratios, widths and trades through EndedValues and returns what went; keyboardResizeDidChange also ends the content hold when nothing moves. DividerDrag sends a release's values through EndedValues too (Option and Command releases set several). - Still two commands for two values: a value the daemon refuses after the other was applied leaves that one applied; atomic commits wait for layout op batches (resize-modifiers.md). Red: the commit before this one (job id in the PR body). Green: job id in the PR body.
…rn is a press (red) Codex review 3 of #34123 at 035fa46 (FIX): EndedValues.send read a missing override as a refusal, but a handler that settles the last width and applies the daemon's snapshot in that turn (a synchronous source, such as MockLayoutSource) removes the overrides because the trade succeeded; PaneResize.press then said nothing moved. Red: job id in the PR body.
…om missing overrides Codex review 3 of #34123 at 035fa46 (FIX): a handler that settles the last width and applies the daemon's snapshot in that turn removed the trade's overrides because it succeeded, and EndedValues.send read that as a refusal. LayoutModel.rejectTransaction now notes a refusal of the transaction going out (TransactionReplies.beginSending/forget/ endSending); send stops at it and returns whether it happened, and an applied change is a sent one. A synchronous refusal of any value, the last included, still stops the rest. Red: the commit before this one (job id in the PR body). Green: job id in the PR body.
…ne's refusal; last-value, unrelated and no-app cases (red) Codex review 4 of #34123 at bf4dfe9 (FIX): TransactionReplies kept the change being sent in one slot, so a second change sent from inside the first one's intent handler took it over and a refusal of the first after that was lost: the first sent its other values and said they went. New file EndedValuesTests: the nested case (red), and guards for a refusal of the last value, a refusal of another transaction, and no intent handler. Red: job id in the PR body.
…n another goes out inside it Codex review 4 of #34123 at bf4dfe9 (FIX): - TransactionReplies keeps the refusal state per transaction being sent, so a change sent from inside another's intent handler no longer takes the outer one's state over; EndedValues checks its own transaction after each emit. - GestureTransactionNumbers keeps each recent transaction's number (the last 32), so a change sent between two values of another does not split that one into two undo steps. Red: the two commits before this one (job ids in the PR body). Green: job id in the PR body.
…usal; a transaction keeps its number while many others go out (red) Codex review 5 of #34123 at d307c16 (FIX): - TransactionReplies kept one refusal entry per transaction, so a change sent from inside another of the same transaction ended it, and a refusal after that was lost. - GestureTransactionNumbers kept the last 32 transactions only, so 32 others numbered between two values of one transaction gave its second value a new number. Red: job id in the PR body.
…n keeps its daemon number while a command of it is in flight Codex review 5 of #34123 at d307c16 (FIX): - TransactionReplies keeps a stack of changes going out, one scope per send (a change can be sent from inside another, even of the same transaction); a refusal marks every scope of its transaction, and EndedValues reads its own (innermost) scope after each emit. - GestureTransactionNumbers keeps a transaction's number while any command of it is in flight (count up per command, down at its answer in sendGesture), with no fixed window: every value of a gesture takes one number whatever goes out between them, and nothing stays once the last command is answered. - EndedValuesTests: the transaction answered in full takes a new number. Not changed (named in the PR): two values are two daemon commits; the daemon refusing the second after the first applied leaves the first applied (atomic commits wait for layout op batches). Red: the commit before this one (job id in the PR body). Green: job id in the PR body.
|
Review (codex): SHIP at c4b8d66 Codex (codex-cli 0.166.0, Codex's answer:
I found no additional demonstrable correctness blocker in the supplied diff. The omitted daemon command scheduling, undo implementation, and snapshot delivery code are not enough to independently establish wire ordering, undo coalescing, or snapshot freshness; those remain dependent on the stated existing contracts and runtime verification. |
…wner-keys-direction
|
Review (codex): SHIP at c4aaf3d Codex (codex-cli 0.166.0, Codex's answer:
No tools or commands used. |
…wner-keys-direction
|
Review (codex): SHIP at aae3728 Codex (codex-cli 0.166.0, read-only sandbox, no tools; the code was in the prompt) reviewed this head: a merge of feat-cmux-next (for the settings-schema base autofix ca66a4c) after its SHIPs at c4b8d66 and c4aaf3d. The PR's own diff against the feat commit it merged (0c3f269) is identical line for line. The pack held that diff and everything the merge brought into CmuxNextLayout and CmuxNextBridge. Green at this head: step 221417dc6bc2d3f47163bf50, 31 of 31 suites. Codex's answer:
|
|
Red checks at aae3728, classified before merge (none is in a file this PR changes):
|
…prit (#34629) * base red: the Chief teleport chip's tooltip is the themed one (NativeTooltipGuardTests, from 894d34f) 894d34f (Chief teleport: location chip, Teleport menu) set the chip's tooltip with toolTip =, which draws macOS's stock bubble; NativeTooltipGuardTests refuses it (Leo 2026-10-12/20: every hover is themed or gone). It is themedToolTip now (TooltipPresenter draws it). Same text. * base red: the All Themes Playground's sample hover is the themed one (NativeTooltipGuardTests, from c0230ef) c0230ef (Status dots keep their hue at 3:1 in every theme; All Themes Playground) put .help( on each sample, which draws the stock bubble; NativeTooltipGuardTests refuses it. It is .themedHelp( now. Same text. * base red: BrowserChromeIconsTests follows 9d31a6e's one app-wide ⋮ (test only) 9d31a6e (one pane ⋮ size and column, Lawrence msg 19627: "the three vertical dots, there are 2 of them, and they are different sizes") made the browser toolbar's More draw the app's action.more.vertical on purpose, with a chrome-icons allow comment. The test still asked for Chrome's icon there. More joins the buttons that keep cmux's icon, and a new case pins its ⋮. * base red: PaneChromeEvenInsetTests follows 9d31a6e's trailing control column (test only) 9d31a6e put the tab bar's ⋮ center on Metrics.paneTrailingControlCenter, the browser toolbar's ⋮ column (Lawrence msg 19627), instead of as far from the trailing edge as from the top. The test still asked for the old equal gaps (off by 2 pt in every density). It now checks the ⋮ center sits on that column. * base red: the status and unread dot tests read the dot's color in the scope that draws it (test only, after c0230ef) c0230ef (Status dots keep their hue at 3:1 in every theme; user 2026-11-03: "we need to make sure every single theme, all the UI elements are colored properly") made Palette.statusUnread/Attention/Error and notificationBlue theme colors: the system hue with its lightness moved per theme to 3:1 on the rows. The views draw them in performWithTheme; SidebarStatusColorTests, HomeSidebarUnreadDotTests and HomeUnreadBlueDotTests still read them outside any theme scope (the bare system color), so every comparison was off. They now read the expected color with the drawing view's performWithTheme. * base red: SidebarLookContractTests reads the hover close on the trailing column, after cad2620's row gap (test only) cad2620 (#32647, Leo 2026-11-02) moved each row's fill in from the sidebar's edge (6 pt compact, 8 pt comfortable) and kept every trailing mark on the grid's 24 pt line on purpose: "Trailing marks stay on the grid's 24 pt column". The close did not move, but the fill's end moved toward it, so at comfortable density the close ends 7 pt inside the fill, not the 8 pt row inset that theHoverCloseIsTheRowsTrailingX asked for (237 > 236.5). The test now checks that the close centers on the row's trailing column and stays inside its row. Other grid tests pin the column itself. * base red: AgentPaneNativeSessionInfoTests closes its windows and carries .closesItsWindows (from #32646) WindowCleanupCoverageTests asks every suite that opens a window to carry .closesItsWindows (#27236). 9c772fa (#32646) and 419a87a (#32704) opened a window in two tests and never closed it. Each now closes its window in its own teardown, and the suite carries the trait. * base red: AgentPaneChromiumMoveTests carries .closesItsWindows (from #32777) 872ef32 (#32777) added a suite that opens a window per test without the trait WindowCleanupCoverageTests asks for (#27236). Each test already closes its window; the suite now carries .closesItsWindows. * base red: AgentShareTests closes the Invite card's window and carries .closesItsWindows (from #33967) 26fd471 (#33967) opened a window for the Invite People card test and never closed it, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). The test now closes its window, and the suite carries the trait. * base red: ColumnStripPageClipTests carries .closesItsWindows (from #32515) 9593a3a (#32515) added a test that opens a window, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). The test already closes its window; the suite now carries .closesItsWindows. * base red: CookieImportConsentPanelTests closes its windows and carries .closesItsWindows (from #33066) 58ec39f (#33066) added tests that open a parent window and a consent panel and never close them, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). The panel only orders out when it is answered. Each test now closes the panel and its parent in its own teardown, and the suite carries the trait. No source change. * base red: DebugNativeRowToolsFixtureTests carries .closesItsWindows (from #28618) 201e1de (#28618) added tests that open windows, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). Each test already closes its window; the suite now carries .closesItsWindows. * base red: InspectElementNativeCostTests carries .closesItsWindows (from 7f2173f) 7f2173f added a test that opens a window, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). The test already closes its window; the suite now carries .closesItsWindows. * base red: the two window-free suites in KeyboardResizeArrowDirectionTests carry .closesItsWindows (from #34123) 4f13a23 (#34123) put KeyboardResizeShownStepTests and KeyboardResizeTradeSettleTests in a file whose other suites open windows. WindowCleanupCoverageTests (#27236) reads the file, so every top-level suite there carries the trait. These two open no window; the trait only checks that. * base red: SidebarPaneGapTests carries .closesItsWindows (from #28052) 5b91230 (#28052) added tests that open windows, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). Each test already closes its window; the suite now carries .closesItsWindows. * base red: the Link Hint suites close their shell windows and carry .closesItsWindows (from #32537) 67a63bd (#32537) gave the Link Hint test rigs a shell window (and one test a page window) that no test closed, in suites without the trait WindowCleanupCoverageTests asks for (#27236). Each test now closes its rig's windows in its own teardown, and both suites carry the trait. * base red: StatusIndicatorHiddenAncestorTests closes its windows and carries .closesItsWindows (from #34074) c35c2e2 (#34074) added a fixture that opens a window no test closed, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). Each test now closes its fixture's window, and the suite carries the trait. * base red: ScreenshotsRememberedAccessTests carries .closesItsWindows (from efacde9) efacde9 added tests that open windows, in a suite without the trait WindowCleanupCoverageTests asks for (#27236). Each test already closes its window; the suite now carries .closesItsWindows. * base red: TabSwitchWorkBudgetTests sizes its strip for 30 tabs at 9d31a6e's minimum width (test only) 9d31a6e made the narrowest tab keep about four title characters: the minimum inactive width went from 48 to 72 pt compact (Lawrence msg 19596: "all here is ... which is not very helpful"). 30 tabs at 72 pt need 2160 pt, so the test's 1800 pt strip scrolled, and the switch scrolled it (occlusion passes 4 -> 75). The test reads the switch budget of a row where every tab shows, so its strip is now as wide as 30 tabs at the strip's own minimum need. * base red: the omnibar's own field editor pads its line by 2 pt from the start (OmnibarFocusLayoutShiftTests, from 15972f3) 15972f3 ("Palette: the empty field's caret sits at the placeholder's left edge") removed EditorAlignedTextFieldCell's fixed 3 pt shift and draws the resting line at the title rect, with the line padding of the field's editor. The palette's editor is AppKit's shared one, already at the field editor's 2 pt padding. The omnibar hands out its own OmnibarFieldEditor, a new text view at TextKit's 5 pt padding until it first edits. So at rest the address drew 3 pt right of where focus puts it (OmnibarFocusLayoutShiftTests: editing x 3, rest x 6), a layout shift on the first focus (user, 2026-10-27: "after focus on omnibar there is layout shift"). The omnibar's editor now starts at the 2 pt padding a field editor has.
… also while its column slides (FRAME OWNER S2B rule D) (#35775) * test(layout): S2B rule D: a grabbed edge holds where it is drawn, in view space (red at fix/frame-owner-s2b-rule-d-red-nomerge) The test commit of the tests-only red branch (6d8d3c8), on this PR's branch. Sidebar-Look: no visual change * fix(layout): S2B rule D: a grab holds the dragged line on the pixel it is drawn on; the pointer alone moves it The refactor owner's decision (rule D, 2026-11-04), from films of a drag during a key spring on the S2B head: under rule C the grabbed line kept its column's slide (C3T column edge at x0.125: +20 pt ahead of a still pointer by rest), and a line caught on the springs' unrounded values sat a pixel off the drawn one (SharedEdgeDuringSpring red on feat since KEYS #34123: a key press moves B's left edge too). At the press (DividerDrag.catchShown, while the screen moves) the drag's value goes to the value that rests the line on the pixel it is drawn on, measured from its anchor's REST place; the pass that lays it out keeps every frame where it shows; then each edge gets its share of the change of speed (minus the line's velocity) that stops the line, by how far it moves with the line (Influence), and the edges that move as the line does also the change of place that puts them on its drawn pixel. The anchor and the edges before it keep their springs. The drag measures the pointer from the drawn line (grab offset and Option's press point moved by the rest line less the drawn one), so the pointer alone moves the line, 1:1, also while its column slides, and where the anchor's rest place leaves the pane less than its minimum the layout clamps the line there: it springs to that place from where it is drawn, no further, and the pointer takes it again past it. No anchor frame is read any more; the clamp fallback (rule C's 0.5 pt check) goes: rule D holds a clamped line by design. The negative-size path check stays. Per-key and per-pass code: unchanged (place()'s guard is as on feat); the change runs at a divider press only. Sidebar-Look: no visual change * fix(layout): S2B rule D: the press probes around where the layout rests the line, so a clamped grab keeps rule D anchorPushesSplitToMinimum (step 710550640fc429cfc23f353e): the caught value was clamped at b1's minimum, both probes (8 pt either side of the drawn line) were clamped too, no influence could be measured, and the press fell back to the old snap, which opened a|b1 97 pt and overlapped b2|c 85 pt. The probes now go 8 pt either side of the line's rest place in the caught layout (the clamp), so one of them moves it. Sidebar-Look: no visual change * test(layout): S2B rule D: a press while the strip camera moves takes the old drag; a pane its spring carries under zero too Codex review FIX at 813ac15 (blocker): stopping the line's own spring cannot hold it where it is drawn while a view offset that places it still moves (the strip camera, a scroll area, a scrolling row). SharedEdgeCatchFallbackTests (new on this branch): a focus scrolls a four-column strip; a press on B's split during the camera's spring must take the old drag. The negative-size case comes from the switch-delete branch. Sidebar-Look: no visual change * fix(layout): S2B rule D: a press whose line's view offset still moves takes the old drag Codex review FIX at 813ac15 (blocker): ShownPixels sampled the strip camera, a scroll area's and a scrolling row's offsets at the press, but their motion went on: a still pointer would not hold the drawn edge. The press now checks whether that offset still moves (ScreenContentView.viewOffsetMoves) and takes the drag from before S2B then. Sidebar-Look: no visual change * refactor(layout): S2B rule D: viewOffsetMoves is SharedEdgeMotion's, not a ScreenContentView member (god type budget) Sidebar-Look: no visual change * test(layout): S2B rule D: a held clamp comes to rest at the clamp; a pointer that passes the clamp before it lands takes the split at once Codex review round 2 at f163471 (blocker): a clamped grab kept the clamp's spring as a leftover, and every later move kept it (the move rule shifts by the target's change), so a pointer that passed the clamp before the spring landed left the split behind it and moving under a still pointer. New case pointerPassesTheClampBeforeItLands (one frame after the press, the pointer moves 30 pt past the clamp); anchorPushesSplitToMinimum now holds 1 s and asserts the split rests at the clamp. Expected: red at f163471 (the passing case). Sidebar-Look: no visual change * test(layout): S2B rule D: the drag rig's helpers in their own file; a camera that moves after the press carries the held line The rig's drag, frame checks, landing and moveLine move to SharedEdgeDuringSpringTests+Drag.swift (the suite file was 620 lines, over the 600 cap). New SharedEdgeCameraLimitTests shows a limit of rule D (Codex round 2, should-fix; the refactor owner's condition 3): a focus that reveals another column during a drag moves the strip camera, and the held line goes with the strip (on screen it moves by the camera's travel, not with the still pointer) until the next pointer event puts it where the pointer says. Expected green before and after the clamp fix (it shows how the code behaves). Sidebar-Look: no visual change * fix(layout): S2B rule D: a pointer that passes the clamp takes the line at once Codex review round 2 at f163471 (blocker): a grab whose drawn line the layout clamps springs the line to the clamp, and every later move kept that spring (each edge moves by its target's change), so a pointer that passed the clamp before the spring landed left the line behind it, moving under a still pointer. The press now records the clamp (SharedEdgeClamp: where the layout rests the line, in the drag's pointer space, which side is free, and each edge's share of the line's move from the press's Influence). The first apply whose requested line is past the clamp drops each edge's share of what is left of that spring (distance and speed), so the line goes where the pointer puts it. Red at c3fffa1 (job 2a56ae90b3dfab8ce6274883): pointerPassesTheClampBeforeItLands, the line 90 pt behind the pointer at the move and still moving 12 frames later. Sidebar-Look: no visual change * test(layout): S2B rule D: the pointer never leaves the line behind on a clamp's spring; a clamped press whose release could show a pane under zero takes the old drag Codex review round 3 at 83583ca: - Blocker: dropping the clamp's spring when the pointer passes the clamp can show the pane on the line's far side under zero: its far edge keeps its own slide while the line goes to the pointer, and the press's negative-size check ran before that drop. New SharedEdgeCatchFallbackTests case: the whole strip slides 300 pt (more than a pane's minimum) with b1 13 pt above its minimum, the split is pressed (it clamps) and dragged to b2's minimum. - The same review asked what holds between the press and the clamp: a pointer that moves toward the clamp faster than the line springs there leaves the line behind it. New cases pointerChasesTheClamp and pointerChasesTheColumnEdgeClamp (B's column edge, B about 40 pt above its minimum width): three frames of a quarter of the way each, then held still; the line must show between the pointer and the clamp. - The camera-limit test's doc now says the next event puts the line under the pointer within the split's limits. Expected: red at this commit (the three new cases). Sidebar-Look: no visual change * fix(layout): S2B rule D: the line never shows behind the pointer on a clamp's spring, and a clamp's release cannot show a pane under zero Codex review round 3 at 83583ca (blocker): the clamp's spring was dropped in one go when the pointer passed the clamp, which could show the pane on the line's far side under zero (its far edge keeps its slide), and until then a pointer faster than the spring left the line behind it. - SharedEdgeClamp.follow replaces the one-time release: each apply drops the part of the clamp's spring the pointer has passed (distance and speed by the same part, so what is left is the same spring, scaled), measured against the line's current target (where the drag's mode rests it now) and the drag's requested line (point - grabOffset, the same in every mode). The hold ends when nothing of the spring is left. - mayShowNegativeSize takes clampLeftover (the line's leftover from its clamped rest after the press): each pane's size along the axis is also followed without its share of it. Springs are linear, so any drop schedule leaves the size between the two checked paths; a press that fails takes the drag from before S2B, as an unclamped press during the same slide already did. Sidebar-Look: no visual change * test(layout): S2B rule D: the camera-limit test moves the camera by a reveal; a focus change waits for the drag's end At d0d3529 (job 199614a02844cf9134f635dc) the test's focus on C did not move the camera (travel 0 pt): the scroll reducer defers a focus reveal while a gesture is active (`reveals: !gestureActive`). What moves the camera during a drag is a reveal or a center request (a tab opened beside a chat, an automation's reveal, Center Column), so the test uses revealColumn(containing:) and its doc says so. Sidebar-Look: no visual change * fix(layout): S2B rule D: the press's clamp check runs for a clamped press only mayShowNegativeSize followed the size without the clamp's share whenever clampLeftover was not exactly 0, which an unclamped press reaches by float rounding (the caught value's layout at scale 0). It now uses SharedEdgeClamp's own 0.001 pt threshold, so a press the layout rests where it is drawn runs exactly the check it ran before. Sidebar-Look: no visual change
|
Swift logic follow-up (tip watcher swift-logic row, STATUS 13027 "SWIFT LOGIC CULPRITS"; lead order). This PR raised
plans/cmux-next/layer-ownership.md says Rust owns every state, list and rule and Swift is the platform shell. Please move the new logic of this PR in a follow-up: to Rust (the crate that owns the area; the owner lists in STATUS 12147-12165 name the target per area), or at least out of the app into its package. If a declaration above is platform code (AppKit, Core Animation, OS calls, strings, tokens, DEBUG tooling), reply here and the table gets a PLATFORM row with your reason instead. The swift-logic row is a report, not a gate: nothing blocks on this comment. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
Resize Pane Left/Right (Ctrl-Shift-H/L, Ctrl-Cmd-Left/Right) on a scrolling strip column with no horizontal split grew or shrank the focused column on its own. On the last column the strip kept that column at the window's edge, so its left edge (the divider a person sees) moved against the arrow, and on a middle column every column after it moved too. A right dock grew with Right, so its inner edge moved left.
Now one press moves exactly one divider one step toward the arrow, as Ghostty's
resize_splitdoes, and changes only the two sides of that divider (lead decision, STATUS 8923; Lawrence asked the lead to decide):ColumnScrollState.keptColumn, for the press's own sync only).ShownResizeRange.trade): it moves whole device pixels from the widths the columns show (a column below its minimum shows at the minimum rounded up to a pixel), keeps the pair's ideal sum so the columns after it and the cut of a column that ends at the window's edge do not change, sends the widths as the daemon keeps them (f32, so its echo moves nothing), and places the result before it sends: exactly the divider moves by the step, or the press is refused. It stops at a column's minimum, the widest share, and the edge of the strip the window shows.ShownResizeRange.dockWidth: its minimum to its share of the window, and a docked dock to the room the strip leaves).A press (and a drag's release) is one change, all or nothing (
EndedValues):LayoutModel.sendsNow, wired fromLayoutHandleMap.sendsNow: a pane the daemon has not made yet, a stand-in, refuses commands). A trade beside a column still being made is refused at once, as other pane verbs on a stand-in are.TransactionReplies), so a daemon snapshot between a trade's two events no longer shows a column at its old width for a frame. This was true of Option and Command drag releases before this PR too.LayoutModel.rejectTransaction), per send scope, so it stops the rest of the change even when another change is sent from inside it; a change the app answers and applies in its own turn counts as sent.GestureTransactionNumbers, kept while any command of the transaction is in flight): the daemon keeps one undo entry per transaction for widths and ratios (cmux-tui-corelayout_undo_coalesces_every_target_in_one_resize_transaction), so Undo Layout Change takes a press back in one step. A dock pin with a width is still two steps (the daemon keeps a dock change apart).Where the rule cannot hold (not changed here):
DockStripGeometry.place): a press on one there moves both inner edges.shownResizeRangeandkeyboardResizeWillChangemoved fromLayoutRootView.swift(over its file budget) toLayoutRootView+KeyResize.swift. Specs:PaneResize's doc comment,plans/cmux-next/terminal-shortcuts.md(Resize pane),plans/cmux-next/column-scroll.md(L6). The keys do not change (no Ghostty-defaults trailer). What's New:release-notes/next/highlights/resize-keys-arrow-direction.md.Testing
Command (every red and green below):
Tests (one new file,
KeyboardResizeArrowDirectionTests.swift): a realLayoutRootViewwith a manual frame tick (every pane's shown frame before and after a press, every frame of a press's spring, the intents sent); geometry walks at 1x and 2x on odd-pixel strips with the daemon'sf32echo; the layout model's settle and reject paths; the workspace content controller (daemon transaction numbers, a stand-in column).Red first, each in its own commit:
bec924e7d90dc1e88a29d9f4FAILED, 8 of 10 tests (Right on the last column moved its left edge 60 pt left; the press scrolled an overflowing strip; a press sent 1 width, not 2; one press took 2 daemon transactions). Scratch ed71b45 (the window-edge test's corrected setup on the base):7e70250dd78848c95adbac43FAILED.137cf14f85b0a76f84a9956aFAILED, those 3 tests.21fb1fb9f7857cc48d484480FAILED, those 3 tests.c047c08d7e8550f6cf1da57aFAILED, those 3 tests.91803501701bcd916ce1b131FAILED, that test.EndedValuesTests.swift: a change sent inside another lost the outer refusal; the daemon number was split by a change between two values):7763e315d73f12efb5108f25FAILED, those 2 tests.69b32b438358729f2a1af32dFAILED, those 2 tests.Green at c4b8d66 (the last commit of the PR's own; the head is a merge of feat after it, with the PR's own diff unchanged line for line): step
55e05374fffc06fe516dcc22(aws-m4pro-1) PASSED, 31 of 31 suites: every new suite, every keyboard-resize suite, the column scroll and close suites, and every divider-drag suite (the release path goes throughEndedValuestoo:DividerModifierDragTests,DragSendsOneIntentTests,DividerDragFollowsPointerTests,DividerDragFrameTickTests,DividerSnapTests,SplitSpringMotionTests,PaneCloseMotionTests).Green at the head aae3728: step
221417dc6bc2d3f47163bf50(aws-m4pro-3) PASSED, 31 of 31 suites. At c4aaf3d (an earlier feat merge) the same 31 suites, stepe4f1bca8cbd13900d50cd231, did not run any test: the lane's web bundle step failed on a feat base red (schemas/settings/settings-schema.json, generated, still names thesettings.cloud.tunnel.*strings #34198 removed; 63 strings missing; the base autofix's Mac exports regenerate it). It runs again when feat is green.GUI proof on a display worker (
--display --label gui-session-v1,CMUX_TEST_REQUIRE_GUI=1, 17 suites): at c4b8d66 stepa62d9b2f7eb83f2e19cc9ac5(cmuxs-Mac-mini-4) PASSED, 17 of 17 suites, 0 skipped; at c4aaf3d step89fa3b3e1b662eba8b923bf2ran no test (the same base red); at the head aae3728 step4de124a758d68f38a0d9e6e7(cmux-austin-mini-0) PASSED, 17 of 17 suites, 0 skipped.Cost of a press (fleet steps
78d133c3ca375a21b3551429and5b77dcf33761de158cb31686, a scratch measurement not in the PR, 2560x1440 at 2x, 300 runs): one key trade'sPaneResize.changecosts about oneScreenGeometry.computeof the screen: p99 0.087 ms at 8 columns (32 panes), 0.302 ms at 32 columns (128 panes), 0.568 ms at 64 columns (256 panes), against the 10 ms action budget.Films (the refactor owner, pixel-perfect): cmux24, 120 Hz 2x, real HID keys, judged per press from the app's input journal and the trace (40 ms before the key, 600 ms after). Verdict GREEN.
aTradeBesideAColumnTheDaemonHasNotMadeIsRefusedAndSendsNothing, the real content controller) is the evidence.TYPING UNDER LOAD (pixel-perfect, typing-load-l6 on cmux18), key-to-commit ms, p50 / p95 / p99 / max:
Reviews: the Claude review subagent said FIX at 3f079dc (the pixel and settle findings, fixed above). Codex (a different model) said FIX at 3f079dc, 7dc25a2, 035fa46, bf4dfe9 and d307c16 (each fixed red-first above), SHIP at c4b8d66, SHIP at c4aaf3d (it checked the merged S2 frame-tick drag path against
EndedValues), and SHIP at the head aae3728 (a merge of feat; it checked what the merge brought into the layout packages). Receipts in the PR comments.Observed / Not run
Observed: the red, green, GUI and measurement steps above; the films at 3f079dc and 7dc25a2 and the typing pair at 3f079dc.
scripts/cmux-next/prepush-gate.pypasses (app_linesgoes down).Not run:
Base and merges: the branch merged feat at dd6fdec (it has #34084) at c4aaf3d (it has #34198, the base red fix for Release builds, and #34199) and at aae3728 (the settings-schema autofix ca66a4c); no conflict any time. The PR's own diff is identical line for line across the three merges.
Area owner told: PL-motion-core (
LayoutRootView+KeyResize.swiftis under the Layout motion row).Changelog
Changed: Resize Pane Left and Right move the divider beside the pane in the arrow's direction in a strip of columns and at a side dock, as in Ghostty
Proof
No capture: the change is in which divider a key press moves; the refactor owner's 120 Hz key films are the evidence.