Repository navigation
fix(sidebar): a row's working spinner draws at the status glyph size - #33120
Merged
Merged
Conversation
Leo 2026-11-03: "i think the spinners are maybe a bit too big in this situation". Every non-dot row badge drew in a 14 pt box, so at compact the working spinner outweighed the 8 pt unread dot and the row's leading icon. A working or loading badge now draws at RowBadgeView.spinnerSide (SidebarStyle.statusGlyphSize: 10 pt compact, 12 comfortable), centered on the same cell; the warning and needs-you marks keep 14 pt. Tests: RowBadgeSpinnerDensityTests (each density and minimal mode: fits the trailing slot, never larger than the leading icon or the warning), RowBadgeMarkSizeTests.theSpinnerIsSmallerThanTheWarning. Sidebar-Look: Leo 2026-11-03 "i think the spinners are maybe a bit too big in this situation" Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-a77eaba095962e4b3835
teamleaderleo
temporarily deployed
to
ci-routing-agent
November 3, 2026 05:52 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
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)
web and size checks: green Shipped in:
|
Collaborator
Author
|
Review (subagent claude-opus-5-5 medium): SHIP at 9598096 Checked: it compiles (access levels, Follow-ups (all go into PR 2, the density-scaled icon tokens):
|
teamleaderleo
added
the
leo-named-fix
|
Found 1 test failure on Blacksmith runners: Failure
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
…r-spinner-compact # Conflicts: # plans/cmux-next/sidebar-owner.md
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. |
teamleaderleo
temporarily deployed
to
ci-routing-agent
November 3, 2026 07:46 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
teamleaderleo
temporarily deployed
to
ci-routing-agent
November 3, 2026 07:46 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…argument table is nonisolated Cherry-picked so CmuxNextAppTests compiles for this branch's fleet steps: since #33120 the test target does not compile at feat-cmux-next on the Linux compile-check (job f88be3b39d1e at 6078fb3) or on the Mac lane (step c4cad3cdb743 at feat 43f59d3): "main actor-isolated static property 'cases' cannot be accessed from outside of the actor" (@test(arguments:)). #33377 lands the same one-line change on feat-cmux-next; merging feat after it lands makes this commit a no-op. (cherry picked from commit 45112a7) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…(cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7)
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…me tick (behind motion.frameOwner.timing, off) (#33725) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
…ld-layout frame (D1, regression of #32635) (#33607) * test(resize keys): one key press sends each terminal one grid, the final one (D1, red) FRAME OWNER REFACTOR S3a. D1 is a regression of #32635 (RESIZE KEYS K3): the sigwinch TUI counts 24 SIGWINCH for 8 key presses on builds with #32635 and 8 without it, and film rk-browser-agent (build 52788515, frame 269) shows the program's start layout (35 columns) for one frame between 38-column frames. The test puts the app's own pane content (PaneContentView, a tab strip over its content host) with a real terminal (TerminalSession, libghostty surface) in each pane of p1 | p2, presses Ctrl-Shift-H or Ctrl-Shift-L on p2, steps the spring to its landing at a fixed 120 Hz dt, runs the landing's after-commit release (LayoutViewContext.afterCommit) and one more layout pass. Each terminal must ask its PTY for the final grid once, at the press, and nothing at the landing, and its surface must never take its start size again after the press. Expected on this commit (no fix): each terminal asks for the start grid and then the final grid again at the landing, and the surface takes its start size at the release (PaneHostView.releaseHeldContent sets only the pane content's frame; endPaneMotion -> releaseSurfaceHold sizes the surface from the terminal host, which still has the start size). Command: cmux-ci run --class isolated --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests --red --ref <this commit> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(resize keys): the one-grid red test covers Ctrl-Shift-K and J too (D1, red) The film's 8 presses include the up and down keys: the same rig with p1 over p2 (a vertical split) presses resizePaneUp and resizePaneDown on p2. Still no fix: every case is expected to fail the same way (start grid, then the final grid again, at the landing; the surface takes its start size at the release). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * base red (from #33377, not landed yet): RowBadgeSpinnerDensityTests' argument table is nonisolated Cherry-picked so CmuxNextAppTests compiles for this branch's fleet steps: since #33120 the test target does not compile at feat-cmux-next on the Linux compile-check (job f88be3b39d1e at 6078fb3) or on the Mac lane (step c4cad3cdb743 at feat 43f59d3): "main actor-isolated static property 'cases' cannot be accessed from outside of the actor" (@test(arguments:)). #33377 lands the same one-line change on feat-cmux-next; merging feat after it lands makes this commit a no-op. (cherry picked from commit 45112a7) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(pane motion): the other two releases of held content send no second grid (D1, red) FRAME OWNER REFACTOR S3a, the refactor owner's review: one test for each other path that lets held content go through PaneHostView.releaseHeldContent. - KeyboardResizeOneGridReportTests.aPaneACloseLetsGoAtRestDuringASecondPressSendsNoSecondGrid: endKeyResizeHold at rest. p1 | (p2 over p3); Ctrl-Shift-L on p1 lands, and before the run loop next waits (where that frame commits and lets go of what it held) Ctrl-Shift-J on p2 springs p2 and p3 and a close of p3 arrives. The close lets go of p1 at rest. - PaneHeldContentLandingTests (new file): motion.pane.holdContent on (set in the shared store, put back after, serialized suite), a host whose size comes to rest a frame before its origin: the landing frame only moves it. Each terminal must send no grid after the final one its spring's start sent, and its surface must never take its start size again. Expected on this commit (no fix): the start grid, then the final grid again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(pane motion): the D1 red tests compile (stored property order, main-actor helper) Compile-check 3ad37799f8ed (ffadef1) failed on two lines of the new tests: Rig set its after-commit closure, which captures self, before its `panes` property was set; and the held-content test's local grid reader was not main-actor isolated inside the generic helper's closure. No change to what the tests check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(resize keys): a held pane content lays out at its rest size before it hears the landing (D1) FRAME OWNER REFACTOR S3a. D1 is a regression of #32635 (RESIZE KEYS K3): one Ctrl-Shift-H/J/K/L press sent each terminal's PTY three grids (final, start, final): the sigwinch TUI counted 24 SIGWINCH for 8 presses on builds with #32635 and 8 without it, and film rk-browser-agent (build 52788515, frame 269) showed the program's start layout for one frame between final-grid frames. PaneHostView.releaseHeldContent set only the pane content's frame (layoutClip), then told the content the motion settled. The pane content (PaneContentView) frames its content host in layout(), so the content host and the terminal host in it kept the start size until the next layout pass; the terminal let its surface hold go from that stale host (start size, start grid) and the next pass sent the final grid again. A Chromium page heard the landing at its stale frame the same way. The release now lays the host's subtree out (a real size in a window only, the rule applyPresentation's per-frame layout follows) before notifyPaneMotionSettled, so every view in the content has its rest frame when it hears the landing. The one content layout is the same work as before, in the same after-commit pass, before the notification instead of after it. All three releases go through this function: the K3 key-resize release after the landing frame's commit, endKeyResizeHold for a pane at rest, and a spring's held content (motion.pane.holdContent, agent panes) at its landing frame. Red, at 94db280 (no fix): step 3789fb93581a3a6a26245771 (aws-m4pro-5), KeyboardResizeOneGridReportTests and PaneHeldContentLandingTests FAILED: 4 of 4 keys and both terminals sent the start grid and the final grid again at the landing, the surface took its start size again (e.g. 540 -> 600 -> 540 pt); the endKeyResizeHold and the holdContent landing tests failed the same way. Earlier red of the key test alone: step 47b106dfc7b2a3d5a929a39e at a6b34db. Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests,PaneHeldContentLandingTests --ref <sha> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(resize keys): a close that takes over a key spring sends the taken-over terminal one grid (D1, red) Review of #33607 (FIX at 519815d): PaneHostView.endKeyResizeHold for a pane still on its key spring sets only the pane content's frame (layoutClip); in the same turn the structural change's applyPresentation calls announceRest, and TerminalSurfaceView.restSize adds the size change to the terminal host's stale held size. The terminal sends a wrong "final" grid at the change's start and holds a wrong surface size; the landing then sends the right grid: two PTY resizes for one split or close during a key spring. aCloseDuringASecondPressSendsEachTerminalOnlyItsFinalGrid (the endKeyResizeHold test, renamed) now also checks p2, the pane the close takes over on the second press's spring (6 frames in): exactly one grid for the close, its final one, and the surface rests at its host size. Expected on this commit (no fix for that branch): p2 sends the wrong grid and then the final one. Also from the review: PaneHeldContentLandingTests injects the hold (PaneHostView.init(holdsContentInMotion: { true })) instead of setting motion.pane.holdContent in the shared TunableStore: the app's hosts get the provider's closure and never read that tunable, and the shared override could leak to suites running at the test's await points. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(resize keys): a pane a structural change takes over mid key spring lays out before the change measures it (D1) Review of #33607 (FIX at 519815d). PaneHostView.endKeyResizeHold, for a pane still on its key spring when a split, close or zoom takes the screen over, set only the pane content's frame (layoutClip). In the same turn the structural change's presentation calls announceRest, and the terminal adds the size change to its host's size (TerminalSurfaceView.restSize), which the key hold had kept: it sent a wrong "final" grid at the change's start, held its surface at that wrong size, and sent the right grid at the landing. Both letting-go paths now share one helper (layOutLetGoContent): layoutClip, then the subtree's layout at a real size in a window, before the content hears anything (notifyPaneMotionSettled for a pane at rest, announceRest for a pane the change takes over). PaneHostView.swift is one line shorter than on the base. Red, at f1f7ae0 (the takeover test, no fix for that branch): step bb3b972222294053e40f7a0f (aws-m4pro-6) FAILED as expected: p2, which a close took over 6 frames into a Ctrl-Shift-J spring, asked its PTY for 73x43 (its size before the first press plus the close's change) and then 66x44; every other test passed (the three paths 519815d fixed). Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=KeyboardResizeOneGridReportTests,PaneHeldContentLandingTests --ref <sha> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 3, 2026
… their curve after a late frame (#33935) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. * Frame owner S2 ON: motion.frameOwner.timing defaults on (springs at each frame's show time, drags once per frame tick) Prepared on PR #33725's head; merge feat after #33725 lands, before the PR opens. The film verdict and the typing-load-l6 pair go in the PR body. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…mit (behind motion.frameOwner.terminals, off) (#33956) * test(motion): red: springs at each frame's show time; drags laid out once per frame tick (FRAME OWNER S2) FRAME OWNER REFACTOR S2 (timing), the failing tests before the fix. Both set the Debug Settings switch `motion.frameOwner.timing` on by its key (the switch comes with the fix; on this commit nothing reads it). MotionClockFrameTimeTests (CmuxNextAppTests): a spring on a window's MotionClock, the window's FrameScheduler fired as its display link fires it. - everyFrameShowsTheCurveAtTheTimeItShowsAcrossARefreshChange: 120 -> 60 -> 120 Hz and 60 -> 120 Hz mid-motion. Each frame must show the spring's curve at the time it shows (link targetTimestamp). Today the clock steps by the time between vsyncs, so after a rate change every frame is one refresh behind (or ahead). - aLateFrameLandsOnTheCurveNotBehindIt: 60 Hz with a frame 3 refreshes late, 120 Hz with a frame 5 refreshes late (film pp-drag-browser: gaps of 35-44 ms). Today a gap over 33 ms steps the springs by 33 ms only (the 1/30 clamp), so the motion falls behind its curve and stays behind. DividerDragFrameTickTests (CmuxNextAppTests, LayoutWorkProbe): five drag events in one display frame, each in its own turn, for a split divider, a column edge and a row edge. Expected: no layout pass between the events, exactly one reconcile and one presentation at the frame tick, at most one overlay sync, one model write (split and column edge), and the divider at the newest pointer. Today each event writes the model and the observation hop syncs it in the next turn (the row edge reconciles in the event's turn): five layout passes between ticks, none at the tick. Fleet red run: see the PR body (the job runs on this pushed commit). * base red: RowBadgeSpinnerDensityTests' argument table is nonisolated (cherry-pick of #33377) The one-line fix of the lead's open PR #33377 (45112a7), so this branch's red and green fleet steps compile CmuxNextAppTests: since #33120 the test target does not compile at feat (RowBadgeSpinnerDensityTests.swift:32, a main-actor static `cases` read by @test(arguments:) off the main actor; red step 54d9cf38e5bea098890e7db8 at d643b86 ended there, no test ran). A merge of feat after #33377 lands makes this commit a no-op. (cherry picked from commit 45112a7) * test(motion): red: a frame after a 0.5 s hang moves the motion by the 0.1 s catch-up limit (FRAME OWNER S2) The second failing test before the S2 fix, added to S2's own new test file (MotionClockFrameTimeTests; the refactor owner asked for it in the design review). aFrameAfterAHangMovesTheMotionByTheCatchUpLimit: 120 Hz, three frames on time, then a frame 0.5 s after the one before it (a main-thread hang), then frames on time. With `motion.frameOwner.timing` on, that frame must move the motion by the catch-up limit, 0.1 s (the stall deadline, also cmux-motion's MAX_FRAME step cap): the motion pauses for the other 0.4 s and goes on along its curve. Today the clock steps by the 1/30 s clamp, so the motion stays 67 ms behind. The helper `misses` takes the time the motion paused for before each frame (none by default, as the other cases use it). * fix(motion): springs at each frame's show time, drags laid out once per frame tick, behind motion.frameOwner.timing (FRAME OWNER S2) FRAME OWNER REFACTOR stage S2 (timing), behind the Debug Settings switch `motion.frameOwner.timing` (MotionTunables.frameOwnerTiming, section Springs), default OFF: with it off, nothing changes. The switch is read on every frame, so one build films the old and the new path; it goes with the old path once S2's films and typing-load-l6 pass. Film pp-drag-browser (feat 5fe33a3, cmux22, 120 Hz 2x): during a real HID drag of a column edge beside two Chromium pages the window's display link ticked 45 times in 1205 ms (8-41 ms apart) while the geometry committed from the 110 mouse events between ticks; springs stepped by the time between vsyncs, at most 1/30 s, so a late frame slowed the motion. 1. Springs at the time each frame shows (Chrome: an animation samples the begin-frame time; a late frame advances to it). - FrameTick gets targetTimestamp (the link's target; a synthesized frame: now) and frameElapsed (from the time the previous frame showed, at most FrameScheduler.catchUpLimit = 0.1 s, the stall deadline and cmux-motion's MAX_FRAME step cap; never back). The first frame from rest shows one refresh, as before. timestamp, elapsed and refreshInterval are unchanged (probes and the film kit read them). - FrameTick.motionElapsed (CmuxNextDesignTokens) is frameElapsed with the switch on, else elapsed. MotionClock (every chrome spring of a window: panes, dividers, cameras, tab strip, sidebar) and the other motion clients (tab-drag ghost, window-close fade, sidebar autoscroll) step by it. - Each frame shows its curve at its target time: the steps add up to the time from the motion's first vsync to the time the frame shows, through refresh-rate changes and late frames. A gap over 0.1 s is a hang: the motion pauses for the rest of it, it does not jump. 2. Divider, column-edge and row-edge drags in the frame tick (Chrome aligns input to the begin-frame; the strip pan's presentPan did this already). - DividerDrag: a move or a key change keeps the newest pointer, keys, Control's trail and the velocity sample, marks them waiting and asks for a frame: no model write, no layout. The release still applies in its own turn at its own point; a cancel drops what waits. - LayoutFrameInput, first in LayoutRootView.advanceFrameBatched (and in landMotion: Reduce Motion, a window out of sight): each screen's waiting drag writes once, the model syncs in the same turn (no observation hop), and while it does the presentations and the overlay sync wait (LayoutViewContext.holdsPresentations). After the springs step, a screen no spring presented presents once, the overlay syncs once (one page geometry post) and onFramePlaced runs once. A drag keeps the ticks for panTickTail frames after its last event (as a strip pan). - The strip pan's per-frame state moved with the new flag into ScreenFrameInput; LayoutRootView.Snapshot is built in one place. Both god types shrink (ScreenContentView 2017, LayoutRootView 2020 -> 2006 lines). Red (the tests before this fix, run on the fleet): - MotionClockFrameTimeTests + DividerDragFrameTickTests at 3df00d3: step 5f0e31d2377cda3b90242005 (aws-m4pro-6) FAILED as expected, 15 issues: from a 120 -> 60 Hz change every frame one refresh behind its curve, a late frame behind and staying behind; 5 drag events in one frame laid the layout out 5 times between ticks and 0 times at the tick, 5 overlay syncs, 5 model writes. - aFrameAfterAHangMovesTheMotionByTheCatchUpLimit at 42966ad: step 8a6f722efebe30a17ecdcb1e (aws-m4pro-5) FAILED as expected (the frame after a 0.5 s hang moved the motion 33 ms, not 100 ms). Green: on this commit, named in the PR body. Area owners told in STATUS (8663): PL-motion-core, PL-main. Sidebar-Look: no visual change * test(next): a terminal resize commit never carries a layer size other than its frame's (red, FRAME OWNER REFACTOR S4) Stage S4 (terminals ready-or-keep, PIXEL-PERFECT PLAN.md): for every terminal in every commit, the layer's size and its pixels change together, or neither changes. Films P2: a host-sized Ghostty surface's view took its new bounds at once and the frame drawn at that size came 2-4 refreshes later, so those commits showed the old frame in the new bounds. TerminalSurfaceCommitSizeTests (new file, CmuxNextAppTests): a real MANUAL_MIRROR surface in a window whose layout() resizes it in AppKit's display cycle; an observer right after Core Animation's commit (CommitBoundary.order) samples, at the end of every main run-loop turn, the surface layer's bounds in pixels and the IOSurface it shows. No clock, no sleep: it waits for commits. - aResizeCommitNeverCarriesALayerSizeOtherThanItsIOSurface: six drag steps; no turn ends with the two at different sizes; the last size shows its own frame. - aReadyFrameIsPresentedInTheCommitThatChangesItsBounds: the first turn with the new bounds shows the frame of that size, and host_fresh_presents grew (the frame was presented in a commit, not by the renderer later). Seam with no effect yet: the Debug switch motion.frameOwner.terminals (TerminalDragTunables.framesOwnedByCommit, default off), which the tests turn on. Expected red on feat: the bounds change before the frame. * fix(motion): S2 review fixes: a reopened window's first frame, zero-time frames keep autoscroll, per-root switch in tests (FRAME OWNER S2) The independent review of d155549 (SHIP, four minor findings): 1. FrameScheduler.invalidate keeps the scheduler of a window that shows again (Reopen Closed Window): it now forgets the last frame's times, so the next first frame steps one refresh, not up to the 0.1 s catch-up limit. 2. A frame that shows no later than the last one (a link frame queued behind a stall frame after a hang) has a motion time of 0: the edge autoscrolls of a tab drag over the columns (LayoutRootView edgeAutoscroll, which the TabDragSession ghost reads), a tab strip drag and a sidebar drag keep ticking on it instead of reading it as "nothing to scroll" and stopping. 3. Drags read the switch through LayoutViewContext.dragsOnFrameTick, which a test pins on its own root (frameTimingOverride, as reduceMotionOverride): DividerDragFrameTickTests no longer sets the process-wide switch across its awaits. MotionClockFrameTimeTests (synchronous) puts back the value it found. 4. Coverage in DividerDragFrameTickTests: a release while an event waits lands at its own point once; Control keeps a quarter of the pointer over several events in one frame; Reduce Motion still writes the drag at the frame tick. Existing tests whose contract the switch changes (only when it is on): DividerModifierDragTests shows a frame after its moves and key changes; DividerCatcherMotionTests expects one frame per drag step with the switch on. Sidebar-Look: no visual change * test(motion): S2 Control trail test changes the keys inside one frame (review of af210bb) Review finding 3: with Control held for all four events, the trail is linear, so the test passed without the per-event trail update in DividerDrag `.moved`. Now two events come with no key and two with Control inside one display frame: the divider must sit 50 pt along (40 + 40 x 0.25). Without the per-event update it shows 20 pt. Test-only change; no app source changes. * GhosttyNextKit: pin 635c6c12a (ghostty-next #69 + #70: the embedder presents a new host size with its bounds) Moves the pin from 34e73ba8b to the release branch merge commit 635c6c12a (release/cmux-9ce9b7d0-e2 = 34e73ba8b + fork PR #69 + fork PR #70): GhosttyNextKit xcframework-635c6c12aaa7d5db895bf8abdb116ca14b1baa20-apple-v6, sha256 475fd4366086f3a9c50e55a0c291c11a12afd80acde57c37225130f981f20826 (SHA256SUMS of release run 41643321585). The URL, the checksum, the ghostty-next gitlink, the notices (package-notices.json, vt-link-graph.json) and docs/ghostty-fork.md move together, as #32997 moved them. #70 adds an opt-in macOS mode (ghostty_surface_set_host_present, ghostty_surface_host_frame, ghostty_surface_host_frame_committed). Nothing changes until cmux calls it (next commits, behind motion.frameOwner.terminals). #69 (one compiled link regex per pattern) comes along; #32997 then pins a commit this one already holds (refactor owner decision, STATUS 9071). * fix(next): terminals ready-or-keep: bounds and pixels change in one commit (FRAME OWNER REFACTOR S4, motion.frameOwner.terminals) Films P2: a host-sized Ghostty surface's view took its new bounds at once and the frame drawn at that size came 2-4 refreshes later, so those commits showed the old frame in the new bounds, and two terminals landed 2 refreshes apart. The waiting present (G1) fixed the order at 4-9 ms of main per drag frame. With the Debug switch motion.frameOwner.terminals on (default off, read on every layout): - TerminalHostView places its surface through TerminalSurfaceView.place: a new size goes to Ghostty (set_size, the grid report as before) and the view keeps its size and old frame on the anchor edge of the wanted frame (the host clips it and paints the terminal background beside it). One size waits at a time; the newest goes next. - TerminalFramePresenter (one per window) runs a pass inside the Core Animation commit of each refresh (a 1-point layer the commit displays, after AppKit's layout and display, as CommitProbe does). Each waiting terminal asks ghostty_surface_host_frame(present: true): a drawn frame becomes the layer's contents and the view and its layer take the new bounds in that commit; one not drawn yet keeps both. The main thread never waits. - Requests of one refresh present together for at most 2 refreshes; a request kept for more than 6 passes takes its bounds anyway (counted). - One refresh after a commit that changed a terminal's shown size, the presenter calls ghostty_surface_host_frame_committed, so echo goes off main again; terminals whose bounds changed without a present (first frame, reveal, fallback) are watched until their frame shows. - Springs ask the rest size once (restAt), drags keep the start size until release, a live window resize stays live: unchanged callers, all through place. Switch off: TerminalHostView sets the frame at once, as before, and Ghostty is never put in the embedder mode. * fix(next): S4 presenter: the pass runs before every commit while a request waits, limits count refreshes, a short landing asks again at once The first green run (step 2e4832c0b1d768df79262667) kept every request: the pass ran only when a request or a frame tick armed it, so a frame drawn between ticks waited for the next tick, and on a host with no display link (stall ticks every 100 ms) the last size of a drag never landed. The commit-display layer hook was also unproven at runtime. - The pass is a main run-loop observer just ahead of Core Animation's commit (order 1,999,999, before waiting and on exit), installed while a request waits and removed when none does. It never wakes the run loop; the frame client's ticks bring a turn each refresh. A size asked during AppKit's display cycle (inside that turn's commit) is presented from the next turn. - The group wait (2) and the keep limit (6) count refreshes (frame ticks), not passes, so a busy turn rate cannot trip the fallback early. - A terminal that lands at an older size than the one it wants asks for the newest at once, in the pass. * test(next): a Reduce Motion key resize never holds the main thread for the terminals (FRAME OWNER REFACTOR S4) Refactor owner, films px-keys-single-3k-s1r-reduce (6 refreshes missed per 4 presses) and px-keys-held-2-s1r-reduce: under Reduce Motion each Ctrl-Shift-H/L lands in its own turn, and that turn ran the terminals' whole resize on main. New case in TerminalSurfaceCommitSizeTests: two real terminals side by side in a real layout (Reduce Motion), presses through PaneResize.press. Measures: - the key turn, timed with ContinuousClock from the press to the end of that turn's Core Animation commit (stamped inside CommitBoundary): median of 4 presses <= one 120 Hz refresh (8.33 ms), every press <= two (16.7 ms); - Ghostty's render counters host_display_passes and host_main_draws do not grow (a display pass of a new size waits up to 4 ms for the frame); - every turn keeps ready-or-keep. * test(next): S4 review: a terminal hidden while a size waits keeps its newest size (red); fixtures restore the switch on a failed setup Review of 114c5ef (SHIP, one switch-ON bug): a host layout of a hidden terminal set its frame but left the waiting request, whose landing then put back the older size; a short landing while hidden stayed at the in-between size. New case aTerminalHiddenWhileASizeWaitsKeepsItsNewestSize. Fixtures set the switch only after their checks and restore it and drawsWhenOccluded when setup fails; first-frame waits are bounded by 10 s, not a turn count; the drag case also checks the presenter's stats (presented > 0, no fallback). * fix(next): S4 review fixes: a hidden terminal drops its waiting size, present and bounds under the CA lock, re-watch after a window move Review of 114c5ef: - place() on a hidden, parked or window-less terminal cancels a waiting size before it takes the host's frame, and requestWantedFrame() of a terminal that is not shown takes the wanted frame at once. - The pass presents (ghostty_surface_host_frame present: true) and gives the view its new bounds under [CATransaction lock], Ghostty's call-site rule: no off-main commit of another terminal can ship the new frame with the old bounds in between. A follow-up size goes after the unlock (set_size posts to the renderer thread). - viewDidMoveToWindow re-watches a terminal in the embedder mode, so the new window's presenter reports its commit and echo goes off main again. - TerminalDragTunables doc: the switch is read on every layout. * fix(next): S4 review 2 nits: request cleared before the landing frame change, the tick's present under the CA lock; the key-turn case is median-only over 6 presses, skips missed boundaries, and fails on missing counters or no presents Review of 3bb310c (SHIP): a request the landing's frame change made for the same view was deleted after it (now cleared before); the tick's present for bounds that already fit takes the CA lock (Ghostty's call-site rule). The key-turn case: a hard 16.7 ms bound per press on a shared Debug host could fail without a fault, so it checks the median of 6 presses only and drops samples whose boundary missed the commit; it fails when a terminal has no render counters, and requires the presenter to show each press's frames. * test(next): the hidden-terminal case lays its host out (it set a width with no layout pending, so nothing ran: steps 9b3e8e1e8573d64c9e186171 and ff0ecbe8cc6c06b15083181f failed the same way on test and fix) * fix(next): S4 receipt review nits: a landing that made a request is not also watched; the key-turn case checks both frames per press and no fallback, and requires render counters Receipt review of 39b7b7f (SHIP): the counters' +1 for a missing set cancelled out (now required once in setup); presented >= +6 over 6 presses passed with one terminal presenting (now >= +2 per press and no fallback); landed() could watch a view that the landing's frame change gave a new request (now not). * test(next): S4 key-turn case: each press waits until both terminals landed (at most 2 s), so its presents count for it Receipt review of c4d64b5: 6 main run-loop turns can pass inside one refresh, so the per-press check raced the renderers (and could count a late press's presents for the next). Test only; app source unchanged. * test(next): S4 key-turn case fails and names the terminal and size when a frame still waits after 2 s (never passes quietly) * test(next): S4 main thread keeps turning when motion.frameOwner.terminals goes on and terminals resize under the recorder's per-commit layer reads (red on pin 635c6c12a) TerminalFrameOwnerMainThreadTests runs a real Ghostty surface in a window on screen (a display worker: CMUX_TEST_REQUIRE_GUI or CMUX_CI_DISPLAY_ROUTED). It turns the switch on after the first frame and resizes the terminal three times, each followed by 2 s of output every 4 ms. A layer reads the terminal layer under [CATransaction lock] inside every main commit, as the film recorder's trace sampler does. A sampler thread pings main every 250 ms; no answer in 3 s samples the process and ends it with exit code 70. Red on the pin 635c6c12a (fork #70): step 2d6513308bb18bf5dff69553 (scratch 73ad54e = 71b9d2f + this test) hung. Its sample: main in CA::Transaction::flush > commit > commit_transaction, waiting on an unfair lock; the Metal completion thread in Ghostty's present > CA::Transaction::pop > commit > commit_transaction, waiting on a pthread mutex (it committed while it held the CA lock: fork #70's embedder branch). The fix is fork #71 (next commit moves the pin). * GhosttyNextKit: pin 7d43bb68b (ghostty-next #71: an off-main present never commits while it holds the Core Animation lock) Moves the pin from 635c6c12a to the release branch merge commit 7d43bb68b (release/cmux-9ce9b7d0-e2 = 635c6c12a + fork PR #71): GhosttyNextKit xcframework-7d43bb68b05e1bad433267fe28cd7205041fc4f5-apple-v6, sha256 12f7ac05eec4a0e0a47daef29545cdaba102e4b02ecafcc825d898ff71f4be2f (SHA256SUMS of release run 41705275916; the downloaded zip matches). The URL, the checksum, the ghostty-next gitlink, the notices (package-notices.json, vt-link-graph.json) and docs/ghostty-fork.md move together, as da28dbc moved them. #71 fixes the P0 hang with motion.frameOwner.terminals on (STATUS 9732): in #70's embedder mode the GPU completion thread committed its transaction while it held [CATransaction lock], and main's commit waited for that lock. Every off-main present now does begin, lock, checks, unlock, commit; the default mode already did, so nothing changes with the switch off. Fork evidence: red run 41694823112, green run 41694827381, Codex and Claude SHIP at 3163f4b73. The previous commit's TerminalFrameOwnerMainThreadTests is red on 635c6c12a (step 2d6513308bb18bf5dff69553) and must pass on this pin. * test(next): S4 a new display scale lands with the frame drawn at it, and the landing's commit is reported (red) Codex review of b9bafbd (BLOCK, bug 1): with motion.frameOwner.terminals on, a backing scale change (the window moved between a 1x and a 2x display) set the layer's contentsScale at once, in its own transaction, while the layer still showed the frame drawn at the old scale. For the 1-4 refreshes until Ghostty's frame at the new scale arrived, the old frame showed at the new scale. That frame then landed through Ghostty's main-thread path, which closes the off-main gate (main_pending), and nothing reported its commit, so the terminal's output stopped going to the screen off the main thread until its next resize. aNewDisplayScaleLandsWithTheFrameDrawnAtIt moves a shown terminal to the other scale (1x <-> 2x) and calls viewDidChangeBackingProperties, as AppKit does. It expects: no sampled turn with the layer's bounds in pixels (bounds x contentsScale) and its shown frame at different sizes; the frame came through the presenter; then output goes off the main thread again within 2 s. The test seam: TerminalSurfaceView.backingScaleOverride (nil in the app: AppKit's convertToBacking and window.backingScaleFactor, as before) sets the scale the view converts its points with. No behavior change while it is nil. Red command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests * fix(next): S4 a new display scale lands with the frame drawn at it, through the presenter, and its commit is reported Codex review of b9bafbd (BLOCK, bug 1). With motion.frameOwner.terminals on, a backing scale change now goes through the same ready-or-keep request as a size change: - updateContentScale gives Ghostty the new content scale at once, but the layer keeps the contentsScale of the frame it shows (holdsScale) while that frame is shown (not NONE or HIDDEN). - backingDidChange (viewDidChangeBackingProperties, didChangeScreen) and the reveal in syncWindowState then call requestPresentIfNeeded: a pixel size the shown frame does not have waits for the presenter. Ghostty refuses the new-size frame on its own (bounds x old contentsScale != its size), so the presenter's pass presents it and landRequestedFrame gives the layer the new scale in the same lock span. All terminals of a window that moved request in the same refresh, so they land together. - The landing goes into the presenter's commits, so the next refresh calls ghostty_surface_host_frame_committed and output goes off the main thread again (before, the frame landed through Ghostty's main-thread path, which closes the off-main gate, and nothing reported it). - A size or scale that waits and gives way (switch off, window change, hidden, no window) gives the layer the held scale at once, as before. Also from the same review (risk 2: the lock span calls AppKit's frame setter and the copy-mode cursor sync): setFrameSize no longer syncs the cursor box during a landing; the presenter does it after the lock span (frameDidLand). ghostty_surface_grid_metrics and ghostty_surface_has_selection take the terminal's renderer_state mutex, so they no longer run under the CA lock. The span now holds only ghostty_surface_host_frame(present: true), the frame and layer geometry and the frame-change counter (a leaf lock). With the switch off nothing changes: holdsScale and requestPresentIfNeeded return at once (embedderPresents false), and syncCursor runs in setFrameSize as before (landing is never set). Green command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg=suite --arg=Packages/macOS/CmuxNext --arg=TerminalSurfaceCommitSizeTests * test(next): the S4 hang case fails on a main-thread stall over 3 s that ends, not only on a hang, and prints its phase timings Step df7d7c86adc1556f1f55b5ec (head 30df24f, cmux14) passed after 55 s, though the case needs about 6 s: its sampler saw main not answer for over 3 s and ran /usr/bin/sample, but main came back and the run ended before the sample did, so the case passed and the stack was lost. The same case at b9bafbd (step 0b61836c1fb3b5e4fcbaa678, cmux8) took 6.1 s with no stall. The sampler now: - after the sample, ends the process with exit code 70 only when main is still stopped (a hang, as before); - otherwise writes the sample and the stall's length to stderr, and records the stall with the test's phase (setup, or switch on and resizes); - finish() waits (async) for a sample in progress, so no stall is lost. The case fails on any stall after the switch goes on. A stall during setup (the first surface on a cold worker) is printed, not failed. It also prints the setup time, each resize's time, commit count and longest commit wait, and the presenter's stats. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Leo, 2026-11-03:
Since the 2026-11-01 "every icon is still small" follow-up, every non-dot row badge (
RowBadgeView) drew in a 14 pt box, so at compact density the working spinner outweighed the 8 pt unread dot and the row's leading icon.Change
RowBadgeView.spinnerSide=SidebarStyle.statusGlyphSize(10 pt compact, 12 comfortable): the same box the shared indicator already takes on badge-less rows. It stays centered on the same cell, so nothing moves when work starts or ends.plans/cmux-next/sidebar-owner.mdentry.Tests
RowBadgeSpinnerDensityTests: at each sidebar density × minimal mode, the spinner fits its trailing slot, is no larger than the leading icon, the status glyph or the warning, and stays centered.RowBadgeMarkSizeTests.theSpinnerIsSmallerThanTheWarningreplacestheSpinnerMatchesTheWarning(the design Leo just asked to change).Observed / Not run
prepush-gate.py(no new violations).Builds on #32699 (group icon size) and #32647 (row gap); neither is undone. PR 2 (density-scaled icon tokens) follows on top.
Changelog
Fixed: the sidebar's working spinner is no longer larger than the row's other marks.
Agent session: agent session
🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Shrinks the sidebar's working/loading spinner to match the row's status glyph size so it no longer outweighs the leading icon and unread dot at compact density.
SidebarStyle.statusGlyphSize(10 pt compact, 12 comfortable) on the same cell center; the red warning and needs-you marks keep 14 pt.RowBadgeSpinnerDensityTestsand updatesRowBadgeMarkSizeTeststo assert the spinner is smaller than the warning.Written for commit 8b3d1da. Summary will update on new commits.