Skip to content

cmux NEXT: a closing sidebar row shifts with a viewport re-anchor - #20795

Merged
teamleaderleo merged 4 commits into
feat-cmux-nextfrom
fix/sidebar-leaving-rows-reanchor
Oct 15, 2026
Merged

teamleaderleo merged 4 commits into
feat-cmux-nextfrom
fix/sidebar-leaving-rows-reanchor

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

cx-y2uf5 item 1. When the sidebar was scrolled and a row closed while the list re-anchored its viewport, the closing row stayed at its old frame and jumped one row height (26 px) against its neighbor. SidebarListView+CloseScroll.applyKeepingViewport shifted rowViews, decorations and group lines by the delta, but a leaving row is no longer in rowViews: RowMotion.run animated it to its end frame without tracking it.

Fix

  • A closing row records its end frame in SidebarRowView.closingEnd. The field is cleared on every path where its close recycles it.
  • RowMotion.shift(_:by:) moves rowViews, the closing rows and their ends by the delta, then re-targets each close on the same .move spring. The original close's completion still recycles the row.
  • Small and fast motion only: nothing new animates, and the closing row just keeps its place relative to its neighbors.

Testing

  • Red (test commit 6799b83): fleet suites on aws-m4pro-9, a display host, fail at SidebarCloseWhileScrolledTests.swift:73: (abs(now - gap) → 26.0) < 0.5. A first run on aws-m4pro-4 skipped the test, because that host has no display and motion snaps there.
  • Green, same command at 9e4d985 on aws-m4pro-9: aRowStillClosingShiftsWithItsNeighborsWhenTheViewportReanchors() passes. The step still exits 1 on SidebarInsertWhileDisplayAsleepTests. That's a base red, and it fails identically on the test-only commit; test(cmux-next): a closed sidebar row may park hidden in the list (base red) #20793 fixes it.
  • Release compile passes at 9e4d985.
  • The god-file check (--base origin/feat-cmux-next), check-concurrency and crash_ratchet all pass.

Captures

Real cmux NEXT DEV builds of the merge-base and this head on a capture mini, recorded with debug.window_record at 60 fps. The setup:

  • a 900x600 window with 40 workspaces, the sidebar scrolled about 4.5 rows;
  • close the third visible row, then, 50-200 ms later, a row above the viewport, so the list re-anchors while the first close animates.

What it shows:

  • Base: about 3 frames after the second close, the closing row jumps one row onto its neighbor ("sb 33" over "sb 32"). At app speed this lasts about one frame. With a slowed spring (motion.spring.move response 1.2 s, labelled), a half-faded ghost then sits in the reopened gap for about 8 frames.
  • Head: the closing row fades in its own slot, with no overlap and no ghost.

The "playback 4x slower" clip is slowed in post-processing only. Base and head events differ by up to 2 frames, because socket timing varies.

Open observation, the same on base and head: at the re-anchor frame the rows below the gap dip 10-15 px and glide back. This fix doesn't change that, and it will be a follow-up bead if it's unintended.

Changelog

Fixed: a sidebar row closing while the list scrolls no longer jumps against its neighbor

🤖 Generated with Claude Code


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


Summary by cubic

Fixes a sidebar row that jumps by one row height when a close above the viewport re-anchors it mid-close.

A closing row wasn't part of rowViews, so a viewport re-anchor left it at its old frame while its neighbors shifted with the scroll offset. Now each closing row records its end frame in SidebarRowView.closingEnd, and RowMotion.shift moves the closing rows and their ends with the rows, then re-targets the close on the same .move spring.

  • Adds SidebarCloseWhileScrolledTests covering two closes in quick succession in a scrolled sidebar.

Written for commit e67b448. Summary will update on new commits.

View guided diff Turn on auto-fix

Showcase

still fast light jump

still fast dark jump

still slow light ghost

still slow dark ghost

compare fast light playback4x slow

Full quality: compare-fast-light-playback4x-slow.mp4

compare fast dark

Full quality: compare-fast-dark.mp4

Open the live gallery

teamleaderleo and others added 2 commits October 14, 2026 15:45
cx-y2uf5 item 1: a close above the viewport re-anchors it while an earlier
close's row is still closing; that row stays at its old y and jumps by the
shift against its neighbors. Red on the fleet before the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cx-y2uf5 item 1. A close above the viewport re-anchors it (rows and offset
shift together), but a row still closing from an earlier change was not in
rowViews: it stayed at its old y and kept closing toward its old end, so it
jumped by the shift against its neighbors. A closing row now remembers its
end frame (SidebarRowView.closingEnd, cleared when its close recycles it);
RowMotion.shift moves it and its end with the rows and re-targets its close
on the same .move spring.

Red: fleet step d5ec16f4a3881f553153b4d2 (aws-m4pro-9, test commit 6799b83):
  SidebarCloseWhileScrolledTests.swift:73: Expectation failed: (abs(now - gap) → 26.0) < 0.5

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6b804560-9eda-43b5-8d92-6a52ef4e8369

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Not verified: macOS jobs did not run: compile and app tests were skipped on e67b448da9; ci-status is green without them.

CI passes on e67b448da9 (run 38691726260 attempt 1).
macOS jobs did not run: compile and app tests were skipped: Fast static checks skipped (the macos job needs both to succeed). Re-run CI once they pass, before merging.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

@blacksmith-sh

blacksmith-sh Bot commented Oct 15, 2026

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs

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

@teamleaderleo
teamleaderleo merged commit 06b11c1 into feat-cmux-next Oct 15, 2026
98 of 99 checks passed
@teamleaderleo
teamleaderleo deleted the fix/sidebar-leaving-rows-reanchor branch October 15, 2026 07:56
@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/38702990697 at ed9bbb8 failed: cmux-next generated files.
Those jobs last passed at 04e0ab4. This pull request is one of 5 merged since: #20907, #20795, #20935, #20782, #20847.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant