Skip to content

Keyboard resize rules in Rust: cmux-layout-reducer shown_resize over the app FFI, proven equal to Swift on a 3,203-case corpus (step A) - #35848

Merged
lawrencecchen merged 6 commits into
feat-cmux-nextfrom
swift-logic/shown-resize-range-reducer
Nov 4, 2026
Merged

lawrencecchen merged 6 commits into
feat-cmux-nextfrom
swift-logic/shown-resize-range-reducer

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Move 2 of the swift-logic plan (lead order after 18:00Z; PL-motion-core did not take STATUS 12205): the keyboard resize rules of ShownResizeRange move to cmux-layout-reducer (shown_resize), called through cmux-layout-reducer-ffi (cmux_shown_resize_*). This is step A of the repo's two-step app FFI move (as #34982, #33660): the Rust rules, the C ABI and the parity proof land now; the app keeps calling the Swift rules until the CCmuxAppFFI release that carries the new symbols is pinned. Step B (branch swift-logic/shown-resize-range-ffi-switch, head b83c6bd, ready) then switches Swift to the C calls and deletes the Swift rule copy.

Owner: PL-motion-core (pane geometry, OWNERS.md "Layout motion"); you may take this over.

What changes:

  • ShownResizeRange+Rules.swift: the five rules (ratioRule, columnWidthRule, dockWidthRule, tradeRule, rowHeightsRule) over the values they read from the geometry, the former method bodies with the reads replaced by inputs, same order of operations. ShownResizeRange.swift keeps its API and call sites, gathers the inputs and calls them; the trade's placement check (movesOnlyTheDivider, a ScreenGeometry.compute) stays in Swift. No behavior change.
  • ShownResizeRangeCorpusTests: a seeded corpus (3,203 inputs: 403 ratio, 300 width, 400 dock, 1,200 trade, 900 rows; each kind with at least 152 answers that move) and its answers recorded from the Swift rules on the fleet, floats as IEEE-754 bit patterns, in cmux-tui/crates/cmux-layout-reducer/tests/data/shown_resize_corpus.txt.
  • cmux-layout-reducer/src/shown_resize.rs: the rules in Rust, ported one operation at a time (Swift's min/max on a tie and NaN, rounded() as half away from zero, the column width through f32 as the daemon keeps it), with unit tests per rule.
  • cmux-layout-reducer-ffi/src/shown_resize.rs and the header: five typed C calls, no handle, no JSON, no allocation (catch_unwind, null checks, ERR_NULL/ERR_PANIC with nothing written).

Numbers

swift-logic-lines.py (swift_logic_lines):

  • base feat 2d102f8: 415,764
  • this head (step A): 415,870 (+106: the split adds the gather code; the rules count until step B)
  • step B head b83c6bd (stacked, not in this PR): 415,581. The Swift rules become C calls; the table rows follow rule (c): ShownResizeRange+Rules PLATFORM (each rule one cmux_shown_resize_* call), ShownResizeRange MIXED 15 (its placement check stays Swift). -183 against the base; the row stays red (+363 at this base) because new feat code rose since 15:38Z.

Changelog

none

Observed / Not run

Observed (fleet):

  • Corpus recorded from the Swift rules: step 6f6123fc3742bbec3124c9f6 (aws-m4pro-3) at f0f3cd0 (branch fix/shown-resize-record-nomerge, this tree with record = true; never lands): recordCorpus passed; artifact sha256 5a487a0ca811, the committed fixture.
  • Swift parity at this head 9c0844a: step b6711b8b1458475196017b2f (aws-m4pro-8): ShownResizeRangeCorpusTests and all 17 KeyboardResize* suites, 46 tests, passed.
  • Rust at 4bc90e6 (branch test/shown-resize-ffi-proof-never-merge: this head plus the FFI crate in the cmux-tui workspace, as the R-INPUT S1a proof did; never lands): cmux-tui-rust-check.sh test shown_resize step 2e06bda6c1cd76f246f6eda1: 15 of 15 passed, both corpus tests included (Rust and C calls, bit for bit); clippy step 2546fc85bfe70b8cbe220ea6: green; fmt step a7216178c64c: red only on base files (cmux-app-manifest, cmux-tui-core, cmux-tui link/sync_cli), none in this PR.
  • python3 scripts/cmux-next/prepush-gate.py: no new violations.

Not run:

  • No perf bench: ShownResizeRange has no // perf-hot mark and no row in perf-hot-benches.json; it runs once per resize key press, not per frame. The step B C calls do no allocation on the Rust side; the Swift side builds two small arrays per row press, as today.
  • The app compile of step B waits for the new CCmuxAppFFI pin (it imports the new symbols).

Display fleet run (gui-session-v1)

  • cmux-ci run --class light --display --script scripts/ci/package-test-lane.sh --arg suite --arg Packages/macOS/CmuxNext --arg <18 suites> --ref 9c0844ac2e973b88020c27b8582fda6008efc9ef (the command of S4's display step 538dccf3f6002506fb3b7549, with the suites that exist at this head): step 777a3be61b678960b38cbaef on cmux14.local, display worker, console session: "All 18 suites passed.", exit 0.
  • Suites (Swift Testing tests executed): KeyResizeCoalescedPressesKeepEveryStepTests 2, KeyResizeFirstTickPinTests 2, KeyResizeKeyTurnSurfaceTests 2, KeyResizeReduceMotionKeyTurnTests 1, KeyboardResizeArrowDirectionTests 20, KeyboardResizeContentHoldTests 2, KeyboardResizeFirstFrameTests 1, KeyboardResizeHoldLifetimeTests 4, KeyboardResizeHoldTakeoverTests 2, KeyboardResizeOneGridReportTests 2, KeyboardResizeReflowAfterLandingTests 2, KeyboardResizeRowsEdgeCasesTests 5, KeyboardResizeRowsFollowupTests 3, KeyboardResizeRowsPixelFloorTests 1, KeyboardResizeRowsTests 2, KeyboardResizeShownLimitTests 2, TerminalFrameOwnerMainThreadTests 1, TerminalSurfaceCommitSizeTests 5 (each N > 0; 59 in total).
  • An earlier display step, 16be4bd35823cbdf60ca1ff7, failed only on its filter: TerminalKeyResizeFrameTests exists on Frame owner S4: terminals follow their pane in every frame of a key resize (switch-on path; default stays off) #35470's branch, not at this head, so it ran 0 tests and the execution guard failed it; its other two suites passed.

…s.swift), the geometry gathered in the wrapper; a seeded corpus test (no behavior change)

The keyboard resize rules move to cmux-layout-reducer (swift-logic owner, step A). To prove the Rust
port equal before the app calls it, the Swift rules first run over the values they read from the
geometry: ratioRule, columnWidthRule, dockWidthRule, tradeRule (the move and the two width pairs to
try; the wrapper still places each pair and keeps the first that moves only the divider) and
rowHeightsRule. Each is the former method body with the geometry reads replaced by inputs, in the
same order of operations. ShownResizeRangeCorpusTests records their answers for a seeded corpus
(recordCorpus) and checks every case against the recorded file (corpusMatchesTheFixture); the Rust
test reads the same file.
…or 3,203 seeded inputs (recorded on the fleet)

Recorded by ShownResizeRangeCorpusTests.recordCorpus at f0f3cd0 (branch
fix/shown-resize-record-nomerge: the same tree with record = true), fleet step
6f6123fc3742bbec3124c9f6 on aws-m4pro-3, artifact sha256 5a487a0ca811. 403 ratio, 300 width,
400 dock, 1,200 trade and 900 row cases, each kind with at least 152 answers that move.
…over the app FFI (cmux_shown_resize_*)

The rules of ShownResizeRange+Rules.swift ported one operation at a time (Swift's min and max on a
tie and NaN, rounded() as round half away from zero, the column width through f32 as the daemon
keeps it): ratio, column_width, dock_width, trade (the move and the two width pairs to try) and
row_heights. Unit tests per rule; tests/shown_resize_corpus.rs checks every case of the recorded
corpus bit for bit. cmux-layout-reducer-ffi exports them as typed C calls with no allocation
(cmux_shown_resize_ratio, _column_width, _dock_width, _trade, _row_heights); its test runs the
same corpus through the C calls and checks the struct layouts and null pointers.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 9c0844a: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr35848 -f ref=swift-logic/shown-resize-range-reducer -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: red check: Check the app FFI pin is not red on the base b24aac2; do not merge until green; see lane-rules

Shipped in:

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41930990041 at a7fac9e failed: cmux-next crash ends the app.
Those jobs last passed at a019920. This pull request is one of 3 merged since: #35898, #35848, #35872.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@blacksmith-sh

blacksmith-sh Bot commented Nov 4, 2026

Copy link
Copy Markdown

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpgzcxq0gz/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmps5z5bfd1/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (independent review subagent): SHIP at 9c0844a

Base 2d102f8, head 9c0844a. Correctness first. I did not build or test Swift or Rust; the evidence is CI on this head, the author's fleet steps, and an independent Python port of the Swift rules (below).

Checks

  1. The Swift split is behavior-neutral. I compared old ShownResizeRange.swift with the new wrapper plus +Rules line by line. The inputs, the order of operations, .rounded() / .rounded(.down), the CGFloat-to-Double conversions, the f32 keep, the two candidates in the same order, the trade's floor (uses scale, not pixel), the stripOffset bounds, the dock filter (isTarget = column == column.id, bands filtered in the rule) and the row floor search (pixel scale vs stack scale kept apart) are all the same. The only differences: minimumSize is now computed before the early nil returns (width, dock), and every row's minimum height is computed, not only the two rows the press uses. These are pure reads, so nothing changes; it is a little more work per press. The 17 KeyboardResize* suites passed at this head (fleet step b6711b8b1458, aws-m4pro-8). The log shows corpusMatchesTheFixture ran and passed and recordCorpus was skipped.
  2. The Rust port equals the Swift rules on every branch. I checked each Swift helper it copies: pixelWidth, fraction, roundToPixel, ceilToPixel (with -0.001), pixelHeight, and the constants 0.1/1.0/0.5/0.75/0.4/400/0.01/100...1000. swift_min/swift_max are the stdlib y < x ? y : x / y >= x ? y : x, so ties and NaN behave the same. round is half away from zero, floor is .down, and as f32 is Float(Double). The trade's delta != 0 is true for NaN, as in Swift. Rust differs from Swift only where Swift traps: Int(NaN or out of range) and Int overflow in the row sums and steps, which Rust saturates. Finite geometry cannot reach these. The corpus has no NaN or inf input; the unit tests cover the NaN min/max cases.
    I wrote an independent Python port of the Swift rules from +Rules.swift. It reproduces all 3,203 corpus lines bit for bit. With branch counters it shows that every reachable branch is hit: ratio fit/none; width none, some, and lower at the top share; dock with 1 to 4 sides, floating, docked with 0 to 3 others, the room limit, and upper below lower; trade guard, scale 0, the offset limit, the room limit, no pixels, left and right moves, and candidates that differ (317); rows with a single row, the last row, step 0, the floor search down and up, a row below its floor, low > high (223), fill and scroll in both directions, and the none cases. The only branch with no hit is ratio lower > upper after the fit guard, and that guard makes it unreachable. All three parity tests compare hex bit patterns and fail on a mismatch (#expect / assert_eq!). Each has a case-count floor, so an empty file cannot pass.
  3. The C ABI. The header structs and the five signatures equal the Rust #[repr(C)] types and extern "C" functions field for field (I read them in order). Size and align are tested. There is no allocation and no handle. catch_unwind is used, null pointers return ERR_NULL with nothing written, and the counts are checked. The style is the same as the crate's other modules (ERR_* from lib.rs, # Safety docs, unsafe_op_in_unsafe_fn deny).
  4. Wiring. This head's own tests run in its own CI. cmux-next fast tier (cargo) runs cargo test -p cmux-layout-reducer with no path filter (FAST-TIER PASS sidebar layout reducer; tests/shown_resize_corpus.rs ran). The rust workspaces job cmux-tui/crates/cmux-layout-reducer-ffi ran clippy --all-targets -D warnings and cargo test --workspace in the FFI crate's own workspace, and its 3 shown_resize tests passed (the corpus through the C calls, the layouts, the null pointers). The never-merge proof 4bc90e6 is this head plus 3 workspace files (Cargo.toml/lock and the removed [workspace] line). It is valid evidence for the same sources, but it is not needed: the head's own CI covers it. The PR body says the FFI proof needed that branch; it did not. The corpus is in cmux-layout-reducer/tests/data/, and the FFI test reads it with include_str!. The Swift test finds it through #filePath; six levels up is the repo root, which is correct. The Swift test is in CmuxNextAppTests and needs no wiring.
  5. Count. I recounted with the base copy of swift-logic-lines.py: 415,764 at the base, 415,870 at this head (+106), 415,581 at step B b83c6bd (-183). Step B keeps every input struct and the wrapper of this split and replaces only the five rule bodies with cmux_shown_resize_* calls. So the split is not larger than step B needs. No CI gate counts this number. Acceptable.
  6. Per key. The only caller is LayoutRootView.shownResizeRange from PaneResize.press: once per resize key press, not per frame. It has no // perf-hot mark on the base, and the hook's perf-hot rule acts only on marked functions, so no bench is required.
  7. Reds. The only red this PR causes is Check the app FFI pin in cmux-next checks, as expected for step A. The pinned source df4352d is also behind the base: the same diff lists focus input and rd-ffi line-reader files from other PRs. Pushed commits add no violations is green. The two warn-only style findings (process scan in acpmux cpu_class.rs, and scrollbars in CloudUI, Feed and webviews) are in files this PR does not touch. cmux-next swift test, cmux-next fast tier (Mac), namespace autofix and release-notary are pending, not red.
  8. Risk and owners. risk:normal is correct: no high kind (no auth, policy, migration, release, gate, hook trust or new delete), and risk evidence is green. No OWNERS row covers CmuxNextLayout/Geometry/ (the "Layout motion" row is Views/ScreenContentView+Motion*, +Dock*, LayoutRootView*, PaneHostView, DisplayLinkDriver) or the reducer crates. Telling PL-motion-core is a courtesy, not a requirement.

Findings (none blocking)

  • cmux-tui/crates/cmux-layout-reducer/src/shown_resize.rs:317: row_heights copies the input into out before it knows the answer, so a 0 return ("moves nothing") leaves out overwritten. The header promises "nothing written" only for the error codes, and step B ignores out on 0, so the behavior is correct. To make the contract plain, either copy after stepped returns Some, or say in the header that out is undefined on 0.
  • cmux-tui/crates/cmux-layout-reducer-ffi/src/shown_resize.rs:250: the # Safety text does not forbid out overlapping input->heights. An overlapping C buffer makes a &[i64] and a &mut [i64] alias (UB). Add "out must not overlap heights". Step B passes separate arrays.
  • The new calls return 1 for an answer and 0 (equal to CMUX_LAYOUT_REDUCER_OK) for none, while the crate's older calls return OK on success. Step B correctly checks == 1. One header line saying "0 is not OK here" would stop a later caller from reading OK as an answer.
  • The layout test checks size and align only, not field offsets. The field order is correct by reading; an offset_of! check like agent_cursor_typed.rs would pin it.
  • PR body: the "Observed" list can say that this head's own CI ran both Rust test sets, as described in item 4.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Expected red for the lander: the app FFI pin step of cmux-next checks is red on this PR by design (step A of the two-step FFI path; app-ffi-release.yml publishes from the feat push and its repin PR follows), as with #35307, #34507 and #35828. Step B (branch swift-logic/shown-resize-range-ffi-switch) opens after the repin and takes the review's non-blocking notes (row_heights writes out only on an answer; no overlap of out and heights in the Safety text; the 1/0 return documented beside OK; field-offset layout test).

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026

@teamleaderleo teamleaderleo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review (codex gpt-6.1-sol xhigh, next-review, since=20261104T212538Z): BLOCK at 9c0844a

BLOCK at 9c0844a

Visual verdict: no images; Leo tries the build.

  1. [P1] [blocking] [category:tests] [finding:F1] [outcome:pending] ShownResizeRangeCorpusTests.swift:89 (and the similar calls through line 111) passes CGFloat values to hex(_:), which only accepts Double at line 189. Swift does not implicitly convert CGFloat to Double, so the new test target cannot compile; convert explicitly or overload hex.

  2. [P2] [follow-up] [category:correctness] [finding:F2] [outcome:pending] cmux-layout-reducer/src/shown_resize.rs:56 uses denominator <= 0.0 where the Swift rule guards denominator > 0. For a NaN viewport or gap, Rust falls through and returns NaN bounds instead of Swift’s DEFAULT_WIDTH; the parity tests contain no NaN cases.

Follow-ups:

  • F2: preserve Swift’s non-positive-or-NaN fallback in fraction.

lawrencecchen added a commit that referenced this pull request Nov 4, 2026
…app FFI, step A: Rust port, C ABI, Swift split and parity corpus (no behavior change) (#35886)

* ClosingPanes: its rules over plain numbers (ClosingPanes+Rules.swift), the geometry gathered in the wrapper; the rules ported to cmux-layout-reducer closing_panes with a C ABI (cmux_closing_panes_*); a seeded corpus test in both (no behavior change)

Move 3 of the swift-logic plan (lead order): the springing-close rules of
FRAME OWNER S3c/S3d leave Swift in two steps (the repo's app FFI path, as
#35848). This is step A's code; the corpus of answers is recorded from the
Swift rules on the fleet in the next commit.

- ClosingPanesRules (Swift): the former ClosingPanes bodies with the
  geometry reads replaced by inputs, in the same order of operations; panes,
  scroll areas and row columns are numbers the wrapper hands out. The
  wrapper keeps ClosingPanes' API and every call site.
- cmux-layout-reducer closing_panes: the same rules in Rust (Swift's min,
  max and Sequence.min/max kept, so ties and NaN answer the same), with unit
  tests. The per-frame rects reuses the handle's buffers (no allocation in
  steady state).
- cmux-layout-reducer-ffi closing_panes: one handle per screen, typed calls,
  no JSON; begin and remove return 1 for an answer and 0 for none (documented
  beside OK), outputs written only on an answer, the Safety text forbids an
  out pointer overlapping an input; struct sizes and field offsets tested.
- ClosingPanesCorpusTests (Swift), closing_panes_corpus (Rust) and the FFI
  test replay the same corpus: each call's answer and a hash of the whole
  state after it, bit for bit.

The neighbour lists were already sorted by pane id; the gaps' min and max
were read in Dictionary order (seeded per process) and are now read in pane
order: only the sign of a zero gap could differ, never a value.

* closing_panes: the state hash in the crate and over the C ABI (cmux_closing_panes_state_hash), so step B's Swift test still compares the whole state after each corpus call

* closing_panes corpus: the answers and state of the Swift springing-close rules for 160 seeded scripts, 1,767 calls (recorded on the fleet: step cf0ccca08931af31dadbcbf3 at a5b46f9, artifact sha256 1e485a58030f)

* closing_panes: a tie in Swift's min keeps its first argument (fix the unit test's expectation); clippy: plain boolean forms and Sequence.min(by:) as a loop (no behavior change; the corpus passed through both)

* cmux-layout-reducer closing_panes: a NaN row travel is no travel, as in Swift (red: Rust skips only abs < 0.5, so NaN travels)

* cmux-layout-reducer closing_panes: skip a NaN row travel as Swift does; read the close order with get() (panic ratchet: indexing_slicing 70 > 69)
@lawrencecchen
lawrencecchen merged commit 728ed6c into feat-cmux-next Nov 4, 2026
349 of 364 checks passed
@lawrencecchen
lawrencecchen deleted the swift-logic/shown-resize-range-reducer branch November 4, 2026 22:48
lawrencecchen added a commit that referenced this pull request Nov 5, 2026
…r over the app FFI; the Swift copy goes (step B of #35848) (#35938)

* ShownResizeRange: the keyboard resize rules run in cmux-layout-reducer over the app FFI; the Swift copy goes (step B)

ShownResizeRange+Rules.swift keeps its entry points and maps them to cmux_shown_resize_ratio,
_column_width, _dock_width, _trade and _row_heights (one call per key press, no allocation on the
Rust side; the dock and row arrays are passed in place). ShownResizeRangeCorpusTests now checks
the FFI against the corpus the former Swift rules recorded in step A. Needs the CCmuxAppFFI pin that
carries cmux_shown_resize_*.

* swift-logic table: ShownResizeRange+Rules is PLATFORM (rule c: each rule is one cmux_shown_resize_* call), ShownResizeRange MIXED 15 (its placement check stays Swift)

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 9c0844ac Deployed Nov 4, 2026 by lawrencecchen via namespace autofix #14555
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants