Repository navigation
P0 RESIZE KEYS K4: Ctrl-Shift-J/K and Ctrl-Cmd-Down/Up move a row edge one exact step - #32471
Conversation
…e row edge one whole step (red) RESIZE KEYS K4 (cmux-ad 2026-11-02, P0; film rk-rows on feat 507438d and the dogfood source): in a column of two rows (rows-v1, New Row) Ctrl-Shift-J/K and Ctrl-Cmd-Down/Up on a row's pane moved nothing: PaneResize.change looked for a vertical split in the row's own tree, then a dock band, and found neither. PaneResize.press (CmuxNextBridge) now holds the press for the App handler (the change inside the shown range, the model write), with the new Change.rowHeights; ShownResizeRange.rowHeights(moving:in:by:) is a stub here (never returned): the test fails. (cherry picked from commit 33c8684)
…ow move its row edge one step RESIZE KEYS K4 (film rk-rows): in a rows-v1 column a vertical key press on a row's pane found no vertical split and did nothing. With no vertical split in the pane's own row, the press now moves the edge below that row (above it for the last row) by one step, 5 % of the column height, through RowResize, the same math as a row edge drag (each row keeps its minimum; a press at a minimum changes nothing), and sends one setRowHeights intent, as a drag's release. (cherry picked from commit 81d2674)
… moves the row edge exactly one step The first K4 fix reused the row drag's math (RowResize.heights), which maps a pointer's pixels to permille over the pair's height without the gap between the rows; RowStackGeometry places a row at (height + gap) * p / 1000 - gap, so each press moved the edge 1.85 pt more than the declared step (green step 4695da17, KeyboardResizeRowsTests:51, 4 of 4 presses). A press now adds and removes whole permille (scaled as a filled stack places them): the exact inverse of the placement, one (height + gap) * 5 % step per press, snapped once to the pixel grid. The row minimums (100 permille and the tree's minimum height) stop it; a press there changes nothing. (cherry picked from commit 0ffb845)
|
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)
Default changesThis pull request changes 3 defaults at
Signed off on this commit by Merge receipt: merged as Regenerate the snapshot with Shipped in:
|
|
Review (independent subagent): FIX FIRST at d5d5c04 The normal path is correct: in a filled stack (sum <= 1000) each press trades Findings
Lower severity (fix with the above or as a follow-up)
What I checked and found sound
Fleet
|
… refused heights, never jump, and leave band docks alone (red) Review of #32471 (FIX FIRST at d5d5c04, comment 6583295487): a row stored below the height its panes need showed at that minimum, but the press stepped from the stored value: Down moved the edge up to 168 pt UP, or changed the model and moved nothing. With rows off and heights [1000, 1000] (New Row on a full row) a press sent 1100 / 900, which the model drops (no motion, no refusal). A scrolling stack brought to 1000 jumped by less than a step, and a top or bottom dock with rows lost its up/down dock resize.
…de heights the model takes; band docks keep their up/down resize Review of #32471 (FIX FIRST at d5d5c04): the press now starts from the upper row's shown permille (a row stored below its panes' minimum shows at that minimum), moves one step toward the press only (never the wrong way; nil when nothing would move, so the key refuses quietly), keeps both rows in 100...1000 (setRowHeights takes nothing else: rows off with [1000, 1000] now refuses instead of sending 1100/900), and in a scrolling stack stops at the sum that fills the column (a part step there, then the filled trade). A top or bottom dock with rows keeps up/down on its band extent.
…s own, and a band dock below its widest extent Green + GUI step dc6882a5 at 5d85619: two edge-case tests were wrong, not the code. In the upper-row test p1 sat in a vertical split inside its row, so the press (correctly) moved that split; p1 now sits beside the three-pane stack. The band dock had width 1.0, its widest, so Down was (correctly) refused; it is 0.4 now.
|
Found 1 test failure on Blacksmith runners: Failure
|
|
Review (independent subagent): SHIP at 76e9d24 I read the diff The earlier FIX FIRST items
The 76e9d24 test changes are honest
Row math against
|
|
Base finding, not this PR: |
… placement, so no row key press is dead next to it Review note C of #32471: the floor came from RowStackGeometry.permille(forPixelHeight:), which rounds to the nearest permille, so it could be up to half a permille below the real minimum; a row stored one permille above that floor took an Up press that moved no pixel. The floor now rounds up from (minimum + gap) / (height + gap) * 1000 (times the fill scale). Notes A and B need no code change: the tests pin the behavior #32471 already has.
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Review (independent subagent): SHIP at 9c778d1 9c778d1 is a merge commit. Its parents are the reviewed head 76e9d24 (SHIP above) and feat-cmux-next 069f8c0, and it brings in 54 commits. |
…ey press (review notes A, B, C of #32471) (#32550) * test(next panes): row key presses stop exactly at the filling sum, move a middle row's lower edge, and take no dead press one permille above a minimum (red) Follow-ups A, B, C of the re-review of #32471 (SHIP at 76e9d24, comment 6585271874). A: the scrolling test of #32471 could not fail for the part step; [520, 500] must give [500, 500]. B: a three-row case. C: the row minimum in permille is rounded to the nearest, so a row stored one above it can take an Up press that moves no pixel. (cherry picked from commit ecf6522) * test(next panes): the dead-press test sweeps the minimum content height, so it fails on a floor rounded to the nearest permille (red) The first version (ecf6522, red step e7a1ebcb) passed on the old code: its few pane stacks never hit a minimum that sits just under a half permille. A copy of RowStackGeometry finds 56 such content heights between 64 and 400 pt at a 857 pt column (for example 64.75 pt: floor 114 rounded, 115 real; stored 115, Up sends 114 and no pixel moves). (cherry picked from commit 3f9caa0) * fix(next panes): a row's minimum in permille rounds up from the exact placement, so no row key press is dead next to it Review note C of #32471: the floor came from RowStackGeometry.permille(forPixelHeight:), which rounds to the nearest permille, so it could be up to half a permille below the real minimum; a row stored one permille above that floor took an Up press that moved no pixel. The floor now rounds up from (minimum + gap) / (height + gap) * 1000 (times the fill scale). Notes A and B need no code change: the tests pin the behavior #32471 already has. (cherry picked from commit f83ca3b) (cherry picked from commit ff9dbbe) * test(next panes): at 1x and 2x, row Up presses never die and end with the row at its minimum (red) Review of #32550 (FIX FIRST at 77abc43, comment 6587348634): the floor rounded up from the exact placement can show one device pixel above the minimum (Up refused while one more pixel could move), and at 1x dead presses remain. ShownResizeRange takes the backing scale (LayoutRootView passes the screen's reconciled scale); the floor does not use it yet here, so the test fails. * fix(next panes): a row's floor is the largest stored permille that still shows at the row minimum on the screen's pixel grid Review of #32550 (FIX FIRST at 77abc43): rounded up from the exact placement, the floor could show one device pixel above the minimum (Up refused while one more pixel could move), and at 1x dead presses remained. The floor now replays RowStackGeometry.compute's placement on the screen's backing scale (ShownResizeRange.scale): the largest stored permille whose placed height, rounded to a pixel, is at most the minimum rounded up to one. A few steps from the exact estimate, no layout pass. * fix(next panes): a row key press checks only its own direction's bound, so Up reaches the row minimum where both minimums do not fit Green + GUI step 7d3cdbc4 at 15df074 (KeyboardResizeRowsPixelFloorTests): with tall minimums (both rows need about 426 pt of an 857 pt column) the Up press was refused 0.5 to 2.5 pt above the minimum. Cause: stepped() refused every press when low > high, but high only bounds Down (it keeps the lower row at its minimum); Up only grows the lower row. Not a rounding order: the pixel floor is exact. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
|
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
P0 RESIZE KEYS K4 (cmux-ad 2026-11-02, "pixel perfect ctrl shift hjkl"). In a column with two or more rows (rows-v1, New Row), Ctrl-Shift-J/K and Ctrl-Cmd-Down/Up on a row's pane did nothing (films rk-rows on feat 507438d and on the dogfood source: 4 presses, 0 motion).
PaneResize.changelooked for a vertical split in the pane's own row tree, then for a column dock band, and found neither.Fix: with no vertical split in the pane's row, a vertical press moves the edge below that row (above it for the last row) one step, 5 % of the column height.
ShownResizeRange.rowHeights(moving:in:by:)trades whole permille between the two rows (scaled as a filled stack places them), the exact inverse ofRowStackGeometry.compute, so every press moves the edge(height + gap) * 5 %points; each row keeps 100 permille and its tree's minimum height, and a press there changes nothing. The press sends onesetRowHeightsintent, as a row drag's release.PaneResize.press(CmuxNextBridge) now holds the press for the App handler (app_lines -8).Based on feat-cmux-next 4729158, after #32421 (K1) landed. Commits cherry-picked from the earlier stacked branch fix/resize-keys-rows-3.
Testing
Red first, on the fleet (
scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext ...), compile-checked first:On the earlier branch (same code on the K1 branch):
4c7110360ce6031586c0c741FAILED (KeyboardResizeRowsTests: the first press moved nothing).4695da174e153e514523a0b2FAILED (each press 1.85 pt over the step: it reused the drag math without the row gap). Replaced by the third commit.b3ff1df9b1269bdae00d4ce6PASSED: KeyboardResizeRowsTests 2/2, KeyboardResizeShownLimitTests 2/2, PaneResizeGestureTests 4/4.At this PR's commits:
5507dfe5e6c7103b60fb1c4eat d5d5c04 PASSED. Red 1caf8f8: step00b1b53e90539096f0dcfea1FAILED as expected (the first press moved nothing).KeyboardResizeRowsEdgeCasesTests, code unchanged): stepc76c7d3ce9744b135cc465fdFAILED (4 of 5 tests).2fe13ddd21f6138b91e048e8PASSED; stepb599ecb3f1e640255c355d85FAILED as expected (4 of 5 tests, 9 issues).435bc457d32371945d989106PASSED (CmuxNext with tests, app); green + GUI proof (display worker gui-session-v1, CMUX_TEST_REQUIRE_GUI=1) at the exact head 76e9d24: step3d9875597a2de2ee67df63d9PASSED: KeyboardResizeRowsEdgeCasesTests 5/5, KeyboardResizeRowsTests 2/2, KeyboardResizeShownLimitTests 2/2, PaneResizeGestureTests 4/4.Observed / Not run
No capture: keyboard resize motion is filmed by the RESIZE KEYS kit (cp/resize-keys-kit).
Changelog
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes Ctrl-Shift-J/K and Ctrl-Cmd-Down/Up so a vertical press on a pane in a rows-v1 column moves the neighboring row edge by exactly one step. Previously these presses did nothing because
PaneResize.changeonly looked for a vertical split or dock band in the pane's row.PaneResize.press(inCmuxNextBridge) now handles the new.rowHeightschange and writes onesetRowHeightsintent, replacing the split-ratio and column-width handling inPaneHandlers.ShownResizeRange.rowHeights(moving:in:by:)trades whole permille between the two rows as the exact inverse of row placement, so each press moves the edge(height + gap) * 5%toward the press; a press at a row minimum refuses quietly instead of moving.Edge cases
Written for commit 9c778d1. Summary will update on new commits.