ClosingPanes rules into cmux-layout-reducer (closing_panes) over the app FFI, step A: Rust port, C ABI, Swift split and parity corpus (no behavior change)

Merged
LA
1d ago
+5009-239

Summary

Move 3 of the swift-logic plan (lead order): the springing-close rules of FRAME OWNER S3c/S3d (ClosingPanes: which panes border a closing pane and the gaps it keeps, where each side shows per frame, the camera carries, the panes nothing moves over, a taken-back close's velocities) move to cmux-layout-reducer (closing_panes), called through cmux-layout-reducer-ffi (cmux_closing_panes_*). This is step A of the repo's two-step app FFI move (as #35848): 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/closing-panes-ffi-switch, head 189585c1c88e, ready) then switches Swift to the C calls and deletes the Swift rule copy.

Owner: PL-motion-core (layout motion, OWNERS.md; FRAME OWNER S3c/S3d by Lawrence Chen, #34170, #35140); you may take this over.

Red test first: I read ClosingPanes for a real defect (NaN or negative frames, a kept handle off the pixel grid, row-column scroll off-by-one, left/right rounding asymmetry) and found none that changes behavior: rects are clamped with max(0, ...), pixel placement is the caller's (pixelEdges), and the scroll travel restarts from where it is. So this move is behavior-neutral and proven by a corpus, not by a red test. One order dependence existed: the gaps' min()/max() were read in Dictionary order (seeded per process); they are now read in pane order (the neighbour lists were already sorted). Only the sign of a zero gap could differ, never its value.

What changes:

  • ClosingPanes+Rules.swift (ClosingPanesRules): the former ClosingPanes bodies with the geometry reads replaced by inputs, same order of operations; panes, scroll areas and row columns are numbers the wrapper hands out. ClosingPanes.swift keeps its API and every call site; it gathers the inputs and maps the answers.
  • ClosingPanesCorpusTests: 160 seeded scripts of closes (1,767 calls: 307 begin, 287 settle, 702 rects, 118 carry, 75 settle-carried, 125 row stops, 109 take-backs, 44 drops), recorded from the Swift rules on the fleet. Each line has the call's answer and an FNV-1a hash of the whole state after it (every side's neighbours, gap, place, travel start and end, carry, row travel, close order, awaiting set), so a difference anywhere in the state fails the line. Answers and state as IEEE-754 bit patterns.
  • cmux-layout-reducer/src/closing_panes.rs: the rules in Rust, one operation at a time (Swift's min/max and Sequence.min()/max()/min(by:) kept, so ties and NaN answer the same), with unit tests. rects (per display frame during a close) reuses the handle's buffers: no allocation in steady state.
  • cmux-layout-reducer-ffi/src/closing_panes.rs and the header: one handle per screen, typed calls, no JSON. From #35848's review notes: outputs are written only on an answer; begin/remove return 1 for an answer and 0 for none, documented beside CMUX_LAYOUT_REDUCER_OK (also 0); the Safety text forbids an out pointer overlapping an input; struct sizes and field offsets are tested.

Numbers

swift-logic-lines.py (swift_logic_lines):

  • base feat e60197ebaec8: 415,963
  • this head (step A): 416,213 (+250: the split adds the gather code; the rules count until step B)
  • step B head 189585c1c88e (not in this PR): 415,646, -317 against the base. ClosingPanesRules becomes one cmux_closing_panes_* call per method (PLATFORM row, rule c); ClosingPanes is MIXED 1 (its live-frame "moving" test stays Swift).

Changelog

none

Observed / Not run

Observed (fleet):

  • Corpus recorded from the Swift rules: step cf0ccca08931af31dadbcbf3 (aws-m4pro-4) at a5b46f931e02 (branch fix/closing-panes-record-nomerge, this tree with record = true; never lands): recordCorpus passed; artifact sha256 1e485a58030f, the committed fixture. A first attempt (9332b0b03dcf) was cancelled by the worker (its same-tree cmux-tui build was terminated), not by a test.
  • Swift at 8cb213154507 (the head differs only in closing_panes.rs): step f4501a147d3412e9a3455c03: ClosingPanesCorpusTests, PaneCloseKeptHandleTests, ClosedPaneContentStaysUntilLandingTests, PaneContentPolicyTests, PaneGroundCoversPaneTests: all 5 suites passed.
  • Rust at this head 97eff4cf43db: cmux-tui-rust-check.sh test every_call_of_the_corpus step 77cce0044edb16fff9d4722e: closing_panes_corpus passed (answers and state, bit for bit); test closing_panes step c6c8c577faddb37a00444a42: 5 of 5 unit tests passed; clippy step f3525fd319e219ea600a0198: green.
  • C ABI at ad74896a0198 (branch test/closing-panes-ffi-proof-never-merge: this head plus the FFI crate in the cmux-tui workspace, as #35848's proof; never lands): corpus through the C calls (step 54801c299cc2d36ec92dca35), null and unknown-kind refusals (06cd9119083d47d4cafef4f8), sizes and field offsets (008991d207639346d93eca03): all passed; clippy step 0ded8428bd69816e8e654345: green. This PR's own CI runs the FFI crate's tests in its own workspace.
  • Earlier reds, fixed in 97eff4cf43db: my unit test expected Swift's min(0, -0) to be -0 (Swift keeps its first argument; the rule was right, the expectation wrong); clippy asked for plain boolean forms and a loop for min(by:) (no behavior change; the corpus passed through both).
  • Bench (rects is perf-hot: 4ms motion-frame): PaneMotionBench (CMUX_PANE_BENCH=1) at the base and this head on one worker (aws-m4pro-1), steps 94e030691940801b9534b97d and d0f1866cb1acee3f3a5e6f03: 0 frames over 8.33 ms in both; 30 panes close p95 1.072 vs 1.024 ms, columnClose p95 0.908 vs 0.922 ms, max 3.5 vs 3.3 ms. An earlier unpinned run of this commit on another worker (fbc2ddf437a9, aws-m4pro-1 vs the base on aws-m4pro-3) went over 8.33 ms in column and columnClose; column does not run ClosingPanes, and the pinned rerun is clean, so it was worker load.
  • python3 scripts/cmux-next/prepush-gate.py: no new violations.
  • Expected red: the app FFI pin step of cmux-next checks (step A of the two-step path; #35307, #34507, #35828 landed the same way). The tip shows it until the repin PR lands.

Not run:

  • pane-perf-terminal.py and anim-perf-terminal.py (the motion-frame bench's terminal and Chromium rows, light and stress): not run on this head. Correction (review 6643086790): this head does change the per-frame Swift path a little: rects now builds a [FrameFact] array (with a per-frame moving value it does not use) and a row set, and looks up each neighbour edge linearly where it used a Dictionary; PaneMotionBench on one worker (base 94e030691940801b9534b97d, head d0f1866cb1acee3f3a5e6f03) keeps that in noise: 0 frames over 8.33 ms, 30-pane close p95 1.024 -> 1.072 ms, columnClose p95 0.922 -> 0.908 ms; they are due at step B, which switches the per-frame call to the FFI.
  • The app compile of step B waits for the new CCmuxAppFFI pin (it imports the new symbols).

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

Summary by cubic

Ports the springing-close rules (ClosingPanes: which panes border a closing pane, the gaps it keeps, where each side shows per frame, the carries, take-backs) from Swift to cmux-layout-reducer (closing_panes), exposed through a C ABI (cmux_closing_panes_*) in cmux-layout-reducer-ffi. Step A of a two-step move: the Rust rules, C ABI and parity proof land now; the app keeps running the Swift rules until step B switches it to the C calls.

  • Splits the Swift rules into ClosingPanes+Rules.swift (the former bodies over plain numbers) and ClosingPanes.swift (a wrapper that gathers inputs and maps answers), with no change to the API or call sites.
  • Records a 1,767-call corpus of seeded closes from the Swift rules; the Swift, Rust and C ABI tests replay the same file, comparing each call's answer and a hash of the whole state bit for bit.
  • Ports the rules to Rust, keeping Swift's min/max tie and NaN behavior (a NaN row travel now means no travel, matching Swift); rects reuses the handle's buffers, so steady state allocates nothing.
  • The C ABI is typed with one handle per screen; begin and remove return 1 for an answer and 0 for none, and outputs are written only on an answer.
  • One behavior-neutral change: the gaps' min/max were read in Dictionary order (seeded per process) and are now read in pane order — only the sign of a zero gap can differ, never a value.
  • Reads the close order with get() instead of indexing to keep the panic ratchet (indexing_slicing 70 > 69).

Review fixes (FIX FIRST at 97eff4cf43db, comment 6643086790)

  • Red first: f5fcce2a3a85 adds closing_panes::tests::a_nan_row_travel_is_no_travel_as_in_swift. Fleet step 5301bde55a53ab7c139a6da5 (Linux, cmux-tui-rust-check.sh test a_nan_row_travel_is_no_travel): FAIL, assertion left == right failed at closing_panes.rs:779 (Rust took a NaN row travel; Swift's guard abs(...) >= 0.5 else { continue } skips it).
  • Green: be70ab937504 skips a NaN travel as Swift does (let moves = abs >= 0.5; if !moves { continue }) and reads the close order with get() (the panic ratchet's new indexing_slicing hit at closing_panes.rs:551). Fleet step c52fcf7cae91ab1337219d0a (test closing_panes): 6 tests run, 6 passed. The PR's own clippy (changed cmux-tui packages) job, whose Rust panic ratchet step runs with --compare-base, passes at this head.
  • swift_logic_lines does not change (Rust only).