Repository navigation
Hover rules in Rust, step B: Swift calls the Rust hover rules; the Swift reducers are deleted - #31836
lawrencecchen wants to merge 17 commits into
Conversation
… tests (red: stub bodies) The Swift hover reducers (HoverCardMachine, HoverIntent, the safe triangle CGRect.isAimedAt, HoverCardEscapeGeometry, HoverCardTiming, GroupHoverCardSummary) move to Rust: cmux-motion `hover` and cmux-layout-reducer `group_hover_summary`, with a typed C ABI in cmux-app-ffi (hover.rs, the Hover section of cmux_motion_ffi.h) and the JSON op `group_hover_summary`. This commit has the API, the C forms and the tests, ported from WorkspaceHoverCardBehaviorTests, HoverCardEscapeGeometryTests, HoverCardRowAlignmentTests, HoverCardWindowClampTests, GroupHoverCardSummaryTests and the machine and intent halves of TabHoverCardTimingTests and TooltipHoverIntentTests. The rule bodies are stubs (no effects, zero geometry), so the tests fail: the red step.
…ow card placement and group summary in Rust The bodies of the rules the previous commit tested, ported line for line from Swift with the same inputs and outputs: HoverCardMachine (phases, delays, aiming, bridging, pins, an owner's card), HoverIntent (tooltip warm group), CGRect.isAimedAt (the safe triangle), HoverCardEscapeGeometry (row band, room, placement, zones), HoverCardTiming and HoverIntentTiming (Leo's and Lawrence's tuned values, unchanged), and GroupHoverCardSummary (as typed lines; the app keeps the localized words and the age formatter). Core Graphics rect rules (standardized edges, half-open contains, null on an over-inset) and Swift's min, max and rounding are kept exactly. No Swift change: the app keeps the pinned CCmuxAppFFI until the re-pin; the Swift half (calling these and deleting the Swift copies) follows it.
… are deleted The Swift half of the hover move (one PR with the Rust change, as the FFI rule of 2026-11-02 04:35Z asks). Same public Swift API, so the coordinator, the panel, the tooltip presenter and their tests do not change: - HoverCardMachine keeps the Rust machine's state as the C struct and maps each HoverTargetID to a 64-bit handle; reduce is one cmux_hover_card_reduce call per event (no JSON, no heap for effects). - HoverIntent (tooltips) calls cmux_hover_intent_call; CGRect.isAimedAt calls cmux_hover_is_aimed_at. - HoverCardEscapeGeometry, room, rowBand and edgeGap call the C functions. - HoverCardTiming and HoverIntentTiming.standard read cmux_hover_timing; the tunables' seconds go through cmux_hover_duration_from_seconds. - GroupHoverCardSummary calls the group_hover_summary JSON op once per card show and puts its typed lines into the localized strings. - webviews hover-intent-parity.test.ts reads cmux-motion's timing.rs. Waits for the pre-merge FFI release: the pinned CCmuxAppFFI has none of the cmux_hover_* symbols, so this does not compile until the repin.
…ed value types (REVIEW.md: pure values do not inherit @mainactor)
GitHub Actions
Inactive
|
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 Default changesThis pull request changes 19 defaults at
Signed off on this commit by Regenerate the snapshot with Gallery before/after for
States
Pick other states with a |
GitHub Actions
Inactive
|
Found 4 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
…ent.hover; HoverIntent stays Hashable - HoverIntent.hover passed the mutating `handle` to `map` unapplied, which Swift refuses (review of fe72ef2). - HoverIntent keeps its public Hashable conformance, by hand: the C state struct has none. - GroupHoverCardSummary: the list line no longer shadows `listed` and `more`. - layer-ownership.md: the hover card and tooltip rules are Rust's now; hit tests, the timer and the panel stay Swift's.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Review (claude-review-hover): SHIP at 95443c5 Read-only review of origin/feat-cmux-next...95443c5 (no build or test on this machine). Correctness first. Checked and found equivalent to the merge-base Swift
Fixed in 95443c5 (from my review of fe72ef2)
Nits (not blocking)
Not run by me: Swift compile (the pinned FFI release lacks the new symbols, stated in the PR), Swift Mac suites, perf A/B. Rust: fleet job 8ac118bd8b58417df771344f green (75/75) at b581621, red job 9651cf0dfca75b673f29a7a8 at the stub commit, as the PR states. |
…ce and splits ports on grapheme clusters, as Swift's String and Character do (red: Rust compares bytes and splits on the byte ',')
…s (NFC equality, comma split on grapheme clusters) A move keeps the behavior (lead 2026-11-02): Swift's `Set<String>` treats a precomposed and a decomposed "é" as one value, and `split(separator: ",")` splits on the Character ",", so a comma with a combining mark is no separator. `unique` now compares NFC keys (first value seen is listed) and the port split walks grapheme clusters. unicode-normalization and unicode-segmentation are workspace dependencies already in the app FFI lock (cmux-palette-ranker, cmux-focus-core); no new package reaches the app.
GitHub Actions
Inactive
GitHub Actions
Inactive
…ons (HoverCardHandles, HoverIntentHandles, GroupHoverCardSummaryWire); the logic table classifies the thin hover calls PLATFORM
GitHub Actions
Inactive
|
Review (claude-review-hover): SHIP at 4b073d1 Read-only review of 95443c5..4b073d1, the author's commits only (c4776fd, 4ead4a4, 4b073d1). I skipped the merge f2490c1 of origin/feat-cmux-next (merge base e88c4a5). It changes only the PR's cmux-tui/Cargo.lock (cmux-host-budget) and brings in swift-logic-classes.tsv. My earlier review of the whole PR (receipt at 95443c5) still applies to the files these commits do not touch. c4776fd + 4ead4a4 (group summary Unicode)
4b073d1 (glue split)
Nits (not blocking)
Not run by me: anything. The fleet jobs (red 41fb34bc3f1df61721789c48, green 6d821e84132797854e9b7bcf, clippy 19b1283f2af5a1faabb6a29c, fmt b6392995e2e2a3e64abcadbc) were still running when I wrote this. I did not check the swift-logic-lines counts (base 415723, head 414925). The Swift half still waits for the FFI release, as the PR states. |
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
…safe triangle, row card placement and group summary, with their C ABI (no Swift change) (#31927) * feat(hover): step A of the hover FFI move: the hover rules in Rust, with their C ABI and JSON op (no Swift change) The Rust half of the hover move (B8; #31836 becomes step B, the Swift half, after the re-pin). Same inputs and outputs as the Swift rules it replaces: - cmux-motion `hover`: the hover card machine (phases, delays, aiming, bridging, pins, an owner's card), the tooltip hover intent, the safe triangle, the sidebar row card's placement and zones, and the hover timing tokens (Leo's and Lawrence's values, unchanged, with their history). - cmux-layout-reducer `group_hover_summary`: the group card summary as typed lines, with Swift's String rules (NFC equality, comma split on grapheme clusters); unicode-normalization and unicode-segmentation are already in the app FFI lock. - cmux-app-ffi `hover.rs` and the Hover section of cmux_motion_ffi.h (ABI 4): a typed, batched C ABI (machine as a plain struct, no JSON, no allocation); the JSON op `group_hover_summary` on cmux_layout_reducer_json. The Swift tests of these rules are ported as Rust tests. Proof (fleet, on rust-migration/hover): red 9651cf0dfca75b673f29a7a8 at 9512791 (stub bodies, 63 of 75 failed), red 41fb34bc3f1df61721789c48 at c4776fd (the 2 Unicode tests failed), green 6d821e84132797854e9b7bcf at 4ead4a4 (77 of 77), clippy df9bf645ec81bf01dc66a491 at 4b073d1 (pass). * fix(hover): the duration cast's allow names its reason (the panic ratchet refuses an allow without reason =) * refactor(hover): the suppression bits are literals (no shift for the arithmetic ratchet to count) --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
…ion/hover # Conflicts: # cmux-tui/crates/cmux-layout-reducer-ffi/src/lib.rs # cmux-tui/crates/cmux-layout-reducer/src/lib.rs # plans/cmux-next/swift-logic-classes.tsv
GitHub Actions
Error
GitHub Actions
Inactive
GitHub Actions
Inactive
…a 60-row sidebar sweep through the hover rules (off unless CMUX_HOVER_SWEEP_BENCH=1; the B8 step B perf A/B)
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
changed the title
|
Review (codex gpt-6.1-sol xhigh, next-review): SHIP at 06911b5 SHIP at 06911b5 No actionable findings in the ABI mappings, handle lifetimes or reducer parity. All 16 gallery pairs are identical; native compilation, suites and perf A/B were not executable here. Follow-ups: none |
|
Review (next-review patch-id; carried): SHIP at 8a752f7 The independent SHIP at 06911b5 carries to this head: the PR's own change ( |
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 (next-review patch-id; carried): SHIP at 56a6e6b The independent SHIP at 06911b5 carries to this head: the PR's own change ( |
Independent adversarial review — HQ #6236 / brief 26: avoid per-pointer temporary Set in hover Step BScope / owner: Existing implementation owner @lawrencecchen; reviewed this PR's exact head Source finding
let keep = Set([phaseHolds ? state.target.id : nil,
state.has_last_hit ? state.last_hit.id : nil].compactMap(\.self))
guard ids.count > keep.count else { return }
ids = ids.filter { keep.contains($0.key) }
handles = handles.filter { keep.contains($0.value) }
The Rust machine can reference only the phase target and last hit. For an equivalent small change preserving the current dictionary contents, replace just the temporary collection with scalar comparisons: let phaseTarget: UInt64? = (1...4).contains(state.phase) ? state.target.id : nil
let lastHit: UInt64? = state.has_last_hit ? state.last_hit.id : nil
let keepCount = (phaseTarget == nil ? 0 : 1)
+ (lastHit != nil && lastHit != phaseTarget ? 1 : 0)
guard ids.count > keepCount else { return }
ids = ids.filter { $0.key == phaseTarget || $0.key == lastHit }
handles = handles.filter { $0.value == phaseTarget || $0.value == lastHit }Why this is equivalent: the original set has exactly 0, 1 or 2 distinct IDs. Actual experiment (executed, synthetic Linux/Swift)I wrote and compiled a standalone Swift 500,000 operations per arm with
Reproducer procedure: create the same Acceptance / request for the existing owner
Disposition: a small, isolated performance improvement for the already-owned #31836; no competing branch or PR created. Treat the measured gain as local synthetic evidence until Mac A/B confirms the end-to-end benefit. Linked back from HQ #6236 brief 26. Repro now committed in an independent HQ draft (2026-11-03)The source and acceptance notes are preserved in HQ draft PR #6653: executable Swift probe. Recompiled/reran with Swift 6.2.1 Optional second probe: zero-effect array conversionI also added a standalone effect-buffer Swift benchmark to draft HQ #6653. The source at `HoverCardMachine.reduce` always builds `[buffer.0, buffer.1, buffer.2].prefix(count)` before converting effects, even when `count == 0`. On Debian/Swift 6.2.1 `-O`, one million synthetic calls per arm showed current/early-guard ~22.3/6.9 ns at zero effects, and ~26.5/11.8 ns with an assumed 90% zero-effect workload. The guard must preserve the subsequent `handles.prune(keeping: state)` and ordering. This is a small, source-backed optional follow-up (~15 ns per empty event), not a measured Mac regression; prioritize the scalar handle-pruning improvement and native A/B first. |
Step B of the hover FFI move (B8): the app's hover rules now run in Rust. Step A (#31927) landed the Rust rules and their C ABI, and the re-pins #32391 and #32459 put them in the app's CCmuxAppFFI. This PR switches the Swift types to call them and deletes the Swift copies of the rules. Nothing the person sees should change: the same rules, the same timing values, now in one place.
This PR changes no Rust:
git diff origin/feat-cmux-next...HEAD -- cmux-tuiis empty, and Package.swift's pin is feat's. So the Rust panic step does not apply.What changes
The public Swift API stays the same, so
HoverCardCoordinator,HoverCardPanel,TooltipPresenterand their tests do not change.HoverCardMachinekeeps the Rust machine's state as the C struct and callscmux_hover_card_reduceonce per event (no JSON, no heap for effects).HoverCardHandlesmaps eachHoverTargetIDto the 64-bit handle the rule compares, and converts events and effects.HoverIntent(tooltips) callscmux_hover_intent_call(HoverIntentHandlesholds the target handles).CGRect.isAimedAtcallscmux_hover_is_aimed_at.HoverCardEscapeGeometry(init, zone, room, rowBand, edgeGap) calls the C functions.keepsCard(at:)reads the Rust zone and maps it, with no rule of its own.rowGapstays a Swift constant (0 pt): the C ABI exports no row gap, and moving one design token would need a new step A.HoverCardTimingandHoverIntentTiming.standardreadcmux_hover_timing; the tunables' seconds go throughcmux_hover_duration_from_seconds.GroupHoverCardSummary.makecalls thegroup_hover_summaryJSON op once per card show (GroupHoverCardSummaryWire) and puts its typed lines into the localized strings.webviews/test/hover-intent-parity.test.tsreads cmux-motion'shover/timing.rs, and two comments inwebviews/src/ui/hoverIntent.tsname it. This is this PR's own diff, not the feat merge's: the pages keep a copy of the native hover timing (HOVER_INTENT), and the test that holds the two equal readHoverIntent.swift. That Swift file no longer holds the numbers (they come from Rust), so the test now reads the Rust file. The values are unchanged (450, 500, 200 and 300 ms).plans/cmux-next/layer-ownership.md: the hover card and tooltip rules are Rust's; hit tests, the timer and the panel stay Swift's.HoverCardSweepBench(off unlessCMUX_HOVER_SWEEP_BENCH=1) measures the main-thread cost per pointer move of a sidebar sweep, for the perf A/B below.Swift logic lines
python3 -I scripts/cmux-next/swift-logic-lines.py --repo . --rev <sha> --json:-798 logic lines. The C glue and the JSON wire are their own declarations, classified PLATFORM in
plans/cmux-next/swift-logic-classes.tsv. The bench added since is a test file (not counted).Tests
bun test test/hover-intent-parity.test.tsat the head: 1 pass, 11 expect() calls.python3 scripts/web_lint_prepush.py --base origin/feat-cmux-next --head HEAD, the pre-push hook's and CI'sbun run checkfor webviews) at 06911b5: "web-lint: web lint passed in 24.7s (webviews)".Perf
Mini scheduler A/B, both arms on cmux18,
--measurement, release build,HoverCardSweepBench: arm A the Swift rules (scratch refrust-migration/hover-ab-base: the merge base 7361813 plus the bench), arm B this head. Specsrmhv-b8-ab-r1-a.jsonand-b.json; results here when they end. Main-thread time per pointer move must not rise.Risk
risk:normal. Every hover card and tooltip runs through the new path; the Swift behavior suites above prove the same behavior.Changelog
none
Observed / Not run
Observed: the merge with feat (no Rust and no pin diff), the line accounting, the pre-push gate, the web parity test and web lint.
Not run yet: the Swift compile, the Mac suites and the perf A/B (all queued above). No capture: no UI changes.