Sidebar row face: SidebarMapping calls the Rust rules (step B of #31925; waits for the re-pin)

Closed
LA
5d ago
+2755-265

Step B of a two-step FFI move. Waits for step A, #31925 (the Rust half alone), to land and for the app-ffi re-pin to be on feat. Then this branch is rebased on that feat (dropping its Rust commits, which step A carries byte for byte), gets its Swift compile, its Mac suites and the STRESS perf A/B, and a new review. Do not land before.

The rules that decide what each sidebar row shows (which tab names it, its folder line, its program, its status line, its unread mark, each tab's shortened title) run in Swift today, in CmuxNextBridge/SidebarMapping*.swift. This PR moves them into Rust, in cmux_layout_reducer::sidebar_face, with the Swift tests ported as the Rust corpus, exposes them through a typed C ABI in cmux-layout-reducer-ffi (no JSON), and makes SidebarMapping call it: the Swift copies of the rules are deleted.

Backlog item C2, first slice (rust-migration lead, owner rm-sidebar-mapping). Rule: plans/cmux-next/layer-ownership.md §1.1 and L4/L5.

What moves

cmux-tui/crates/cmux-layout-reducer/src/sidebar_face.rs and sidebar_face/text.rs:

Swift rule (SidebarMapping)Rust
frontTab, selected (window selection, else the clamped default tab; first pane on a focus tie)Workspace::front_tab
workTab (a page yields to the newest chat or terminal; page and app tabs give no name)Workspace::work_tab
labelTab, isIdle, bareShells (an idle shell or New Tab page yields to a chat, a working or waiting one first)Workspace::label_tab, is_idle, BARE_SHELLS
faceTabWorkspace::face_tab
chatTab, folderTab, the Group by Folder bucket (focused?.cwd ?? any tab's)chat_tab, folder_tab, Face::bucket_tab
agentTab (the harness icon lookup reaches Rust as Tab::branded)agent_tab
attentionReason / newestUnread (first on a tie), the unread count or dotnotice, unread
progress(_:tabs:), progress(TerminalProgressReport)progress, terminal_progress
kind, rowKind, listedKindkind, row_kind, listed_kind
processprocess
statusLine, entry, the ports/pr keys, the status block's entry filterstatus_line, entries_status_line, entry, is_status_entry
abbreviate (inside the row; the per-tab Swift callers keep their copy, see below), abbreviateRemote, shortenedPath, trimmingSlashabbreviate, abbreviate_remote, shorten, shortened_title, trim_slash
lastActivitylast_activity, Face::last_activity_ms
defaultWorkspaceName (Creating)default_workspace_name

face(&Workspace, &Context) -> Face runs them all for one row: O(that workspace's tabs).

What stays in Swift, and why

  • sidebarTitle, isUnnamed: they read the page face (title, favicon), which the app's browser runtime computes for the tab this rule picks. Moving them would need a second call per row or every tab's page title in the request.
  • agentRowTitle, shortSessionName: they belong to the auto-name rules (WorkspaceAutoNameReader, and the daemon's own auto rename); they should move with those. agentRowTitle also uses Foundation's case-insensitive compare (full Unicode case folding), which str has no exact match for.
  • pageHost: Foundation URL host parsing; a hand port would differ on IDN and edge cases.
  • detail/activitySince (+Detail), presence (+Presence): they read acpmux's turn store, StatusMapping and SidebarTunables; StatusMapping is its own rule set.
  • Group mapping in sections (color tokens, custom #RRGGBB, collapse union): the group color enum is V-29 (one generated enum), not this slice.
  • machineIcon, spaces, creatingRow, icon, color, checklistItem, taskStatus: conversions into the app's drawing types.

Findings (differences found while mapping)

  1. The backlog names cmux-layout-reducer/src/rows.rs as the Rust owner of sidebar rows. It is not: rows.rs holds the rows of panes inside a layout column (InsertRow, MoveTabToRow). No SidebarMapping rule existed in Rust before this PR.
  2. sidebar_doc/view.rs computes only the structure of the rows (keys, sections, groups, collapse, filter). It takes the row title as an input (LiveWorkspace.title), which Swift computes. The typed rows reply of the mirror (view_typed, typed.rs) carries no workspace content, so this PR adds its own typed call instead of extending that reply.
  3. The daemon's Mux::default_workspace_name (cmux-tui-core mux/workspace_name.rs) is a second copy of defaultWorkspaceName that differs: it parses the number with usize::from_str, which takes workspace-+7 as 7 (Swift and the new Rust rule take ASCII digits only), it accepts numbers up to u64::MAX (Swift's Int stops at i64::MAX), and it knows no reserved names. Not changed here (that would change the daemon); a follow-up should make the daemon call this rule.
  4. Swift compares strings by canonical equivalence; Rust compares bytes. A title and a folder that are the same text in NFC and NFD were equal in Swift (process, isIdle, shortenedPath, abbreviate's prefix) and differ here. Both come from the same file system bytes in practice. Swift's split(separator: "/") in abbreviateRemote splits by grapheme, so a / followed by a combining mark was not a separator; Rust splits at every / byte.
  5. process, isIdle and folderTab shorten against this Mac's home even on another computer's row (Swift calls abbreviate, not the machine's shorten). So on a Cloud row, an idle shell at its home (/home/cmux) still gives a chat-named row its ~ folder line, which the same shell at this Mac's home does not. Kept as it is (no behavior change); a candidate fix for the Swift half's follow-up.
  6. defaultWorkspaceName adds 1 with Swift's trapping +; Rust saturates at i64::MAX.
  7. Progress values are not clamped in Rust: Bar.value and progress_value can leave 0...1 for an out-of-range report. Swift's SidebarProgress init clamps them, and the Swift half keeps that clamp. CTab.progress_value marks no value with CMUX_SIDEBAR_FACE_NO_VALUE (INT32_MIN), so a negative report reaches that clamp as Swift passes it today.
  8. The public Workspace model reads tabs with get(): pane tab counts that miss the tabs give no tab, never a panic (the FFI also refuses such a request).

The Swift half

  • New CmuxNextBridge/SidebarFaceFFI.swift (175 lines, classified PLATFORM in plans/cmux-next/swift-logic-classes.tsv: "thin FFI call to cmux-layout-reducer rows"): it encodes one workspace's tabs (O(its tabs)) into the C request, makes one cmux_sidebar_face_rows call and copies the flat reply out (the C row struct, the listed tabs, the kept entries and the string bytes). There is no Swift mirror of the face: SidebarMapping.row reads the imported C row fields. The C structs themselves are generated: Swift imports them from cmux_layout_reducer_ffi.h. The hand-written part is only the field-by-field read of TabModel into CmuxSidebarFaceTab (16 lines). Its one state is the Rust handle, which owns and reuses the reply buffers, so the handle is used on the main actor only and each reply is copied out before the next call. The handle is freed in the deinit. A Level says what a call encodes: the thin calls (below) read only what their rule reads, so an observing caller (the auto-namer, each window's titlebar) is not woken by a tab's progress, notification or agent report; the row map encodes every field.
  • SidebarMapping.row takes every rule above from the face and keeps only what reads app-only state (page face, StatusMapping, the turn store, presence, detail, icons, colors). frontTab, workTab, faceTab and rowChat stay as public thin calls of the same face (the auto-namer, the titlebar and Copy Agent Identifier use them); defaultWorkspaceName calls cmux_sidebar_face_default_name.
  • Deleted: SidebarMapping+ChatFront.swift (labelTab, isIdle, bareShells, folderTab), and in SidebarMapping.swift selected, agentTab, agentBrand, attentionReason, newestUnread, lastMessage/chatTab, kind/rowKind on tabs, both progress rules, statusLine, entry, process, lastActivity, shortenedPath, trimmingSlash, abbreviateRemote; the defaultWorkspaceName body in +Creating. The ChatFront row of swift-logic-classes.tsv is removed; the other SidebarMapping extensions keep their rows at their new ordinals (--missing: none).
  • Package.swift: CmuxNextBridge links CCmuxAppFFI.
  • Two Swift tests read rules that now live only inside the face: the agent-tab test reads the row's agentName, and the titlebar test reads faceTab (the call the titlebar makes; its front tab is a terminal, so faceTab and the old labelTab pick the same chat).
  • One fix, not a move drift: Copy Agent Identifier on a workspace (rowChat) copied the wrong chat in one case. Its contract (AgentChatIdentifierHandlers.rowChat: "the chat a workspace row's summary line shows", and CopyAgentIdentifierTests.theWorkspaceRowCopiesTheChatItsSummaryShows) is the chat of the row's summary line. The old row built that line from the label tab's chat (lastMessage(labelTab(front))), but the old rowChat used the front tab's (chatTab(frontTab(...))). With an idle shell in front and a second chat working, the row showed the working chat's last message and the action copied the first chat's id. Both now read the same Rust chat_tab(label). Test: SidebarChatFrontTests.copyAgentIdentifierTakesTheChatTheRowShows (the old rule gives the first chat, s1; the new one the working chat, s2).
  • Observation, named: the thin calls observe slightly more than the old Swift rules did. Every level reads each pane's selected tab (the old frontTab read only the most recently focused pane's), and faceTab/rowChat read the title, folder, command and agent state of every tab of that workspace (the old rule read them for the front tab, and the chats' when it was idle). Both are O(that workspace), and selection changes come from the person; no caller is woken by another workspace or by progress, notification or agent-update events.
  • abbreviate stays in Swift in this PR (base body, unchanged). Its other callers (the tab strip's TabItemMapping.item/title, the hover card, the screen bar) map one tab per call from App code (PaneController, TabTitleResolver), so a Rust call there would be one FFI crossing per tab, not one per change; a batch cannot reach them without changing those App files (TabTitleResolver is in the TYPING UNDER LOAD fix's F6 list). The row map uses the Rust rule inside the face. So the home-prefix rule exists in both languages until the follow-up: a batched abbreviate (one call per pane's tabs) after the typing fix lands. SidebarAbbreviateCostTests therefore still measures the base code.
  • SidebarBridge*.swift (shortcut-latency SL-5) is not changed.

Swift lines (python3 -I scripts/cmux-next/swift-logic-lines.py --repo . --rev REV --json)

base 418f3c73aebhead cc8b828aea4
swift_logic_lines415342415217 (-125)
mirror_lines7052070520
CmuxNextBridge swift_logic_lines54185293
SidebarMapping declarations (LOGIC)789664

Swift sources: 255 lines removed, 300 added (the 175-line PLATFORM glue and the new test). An earlier head of this PR (1a13fecc137) had a 317-line hand-written glue that, unlisted, counted as logic (+170); the lead asked for this smaller one.

The C ABI

cmux_sidebar_face_new/free/rows/default_name in include/cmux_layout_reducer_ffi.h: fixed-size request structs (workspaces, panes, tabs and status entries as runs of four arrays, byte ranges into one UTF-8 buffer) and a reply the handle owns (one face per workspace, one listed tab per tab, the status block's entry indexes, one string table). One call per frame takes every row the app's row feed marked changed, so the cost stays O(changed rows) for the 10k-row budget. A listed title is copied only when it changed. Each face also carries the work tab (workTab), which the app's page face and auto-namer read. A request with a range out of bounds, non-UTF-8 bytes or an unknown code answers ok 0 and no faces; the call never panics across the ABI.

Expected red: 'Check the app FFI pin' in 'cmux-next checks' (check-app-ffi-pin.sh), until app-ffi-release.yml publishes cmux-app-ffi-<sha> from feat-cmux-next and its re-pin PR lands, as on #29440.

Perf A/B (required before landing; runs after the pre-merge FFI release)

The row map now crosses the FFI once per changed row and copies each tab's title and folder into the request; the thin faceTab/frontTab/workTab/rowChat calls (the titlebar, the auto-namer) encode only their rule's fields, and the A/B measures them with the row flush. Plan: base and head builds on one exclusive capture mini (capture-host acquire --slot --exclusive, one lease per arm, arms interleaved base, head, base, head), the STRESS session (scripts/cmux-next/perf-stress-session.py) and a light session; metric: main-thread time per sidebar row flush (SidebarRowFeed flush) p50/p95/p99/max and the structural pass. Gate: head p95 and p99 not above base. Not run yet: the head cannot build until the release exists.

Tests

Commit 4fa11043add ports the Swift tests (SidebarAbbreviateCostTests, SidebarTabHomeTitleTests, SidebarChatFrontTests, SidebarWorkTabTests, SidebarWorkTabIconTests, SidebarRowPageBesideChatTests, and the row facts SidebarMapping.row derives) with stub rule bodies; commit f1e2f7d1a9d moves the rules; 7e0017bee42 adds the C ABI, edcc88cff8d its work tab and abbreviate, 5e567776f7b the review fixes (panic-free tab reads, a clippy fix, the FFI error reply written before its pointer is returned, the progress no-value marker), and cc8b828aea4 removes the abbreviate C entry again (no batched caller yet).

scripts/ci/cmux-tui-rust-check.sh test sidebar_face (Linux, fleet):

  • red at 4fa11043add: bd28bcc956daaeb47b9f116e, failed as expected: 38 tests ran, 34 failed on their assertions; the 4 that pass are the cases whose title stays unchanged (a renamed tab, an agent's tab, a title that is not its folder), which the stub also returns
  • at 5e567776f7b (same Rust as the head): test 17660fb49dae7b9362d52c7a green; clippy 32a80ddeb4b2e369849818c5 and panic ratchet 6c5b0610323fdf9cbf56d5e5 red only on base findings (clippy: cmux-tui/src/plugin_manager.rs:233 suspicious_open_options, fixed on feat by #31697; panic: acpmux indexing_slicing 697 > 686). cmux-layout-reducer has no clippy finding and no deny hit (its arithmetic_side_effects warning count is 61 against 58).
  • the FFI crate's tests and clippy, on the proof-only ref rust-migration/sidebar-mapping-ffiproof (50f2cef7dc0: this branch plus one never-merge commit that builds cmux-layout-reducer-ffi in the cmux-tui workspace): test dd4295424aa1cee7f5549678 green, 44 of 44 (the 39 rule tests and the 5 sidebar_face_abi_* C ABI tests); clippy eaa22f00cb22fa660b10c55a checked cmux-layout-reducer-ffi with no finding, red only on the same base plugin_manager.rs:233. Rerun after the feat merge on rust-migration/sidebar-mapping-ffiproof2 (a3371c83dcb, same Rust as the head): test 88c213f03a3efecc9f080d45 green (44 of 44), clippy d7aca0703ba590e5e0b733eb green (the whole cmux-tui workspace with cmux-layout-reducer-ffi in it, -D warnings; the base red is gone after the feat merge). At the head (Rust change since: the unused cmux_sidebar_face_abbreviate entry is removed) on rust-migration/sidebar-mapping-ffiproof4 (69f772cdddd): test 1c19d178401f2ce9ce16d223, clippy 1956528240165ea00f5b47b2 (running).

cmux-layout-reducer-ffi is its own Cargo workspace, which the fleet script does not build; its tests (tests/sidebar_face.rs: struct sizes against the header, a two-row call, undecodable requests, the default name) and its clippy run only in the cmux-tui workflow's lint job (step "cmux-layout-reducer-ffi (own workspace)"), which did not run on this PR. No fleet script builds that crate, so they have not run yet.

Observed / Not run

Observed: the fleet steps above; python3 scripts/cmux-next/prepush-gate.py (no new violations, arch ratchet included).
Not run: no Swift compile, no Swift suites and no perf A/B: they need the pre-merge FFI release (the pinned CCmuxAppFFI has no cmux_sidebar_face_*). After the release: cmux-ci compile-check --app --package CmuxNext, then on a Mac (proof-step.py --suite) SidebarTabHomeTitleTests, SidebarChatFrontTests, SidebarWorkTabTests, SidebarWorkTabIconTests, SidebarRowPageBesideChatTests, SidebarQuietUnnamedRowTests, SidebarDefaultWorkspaceNameTests, SidebarAbbreviateCostTests, SidebarAgentRowPresentationTests, RapidWorkspaceCreateTests, then the perf A/B above. No local cargo or swift on this VM (fleet only).

Overlap: the P0 TYPING UNDER LOAD fix (agent typing-fix) works on SidebarMapping's main-thread cost; it lands first, and this PR rebases on it and keeps its tests green (STATUS 5117).

Changelog

Fixed: Copy Agent Identifier on a workspace copies the chat whose last message its sidebar row shows, also when an idle terminal is in front.

Risk: high (every row title and the sidebar's perf budget). No intended behavior change; the differences found are listed above.