Skip to content

cmux-next sidebar: the list matches a fresh rebuild after each operation (invariant test, rails in list order) - #23389

Merged
teamleaderleo merged 3 commits into
feat-cmux-nextfrom
cmux/sidebar-rebuild-invariant
Oct 21, 2026
Merged

teamleaderleo merged 3 commits into
feat-cmux-nextfrom
cmux/sidebar-rebuild-invariant

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

#23182 was one bug of a class: an incremental sidebar path drifted from what a full rebuild draws, and the drift stayed until a rebuild. This adds one invariant test for the class, and fixes the one drift it found.

The invariant

SidebarIncrementalMatchesRebuildTests runs seven operations on a real SidebarView, through the paths the app uses (the sidebar document's drag, the model's fold, a click, an external tab drag), then the daemon's echo:

After each step, and after every group then folds and opens and a few rows are clicked, it compares the list on screen with a list built fresh from the same model: rows, their order and frames, row views (frame, alpha, selection, untracked strays), drawn rails, the list's rail records, selection, suppressed rows and the gap. Motion is off (Reduce Motion), so the list shows the spring's ends. A mismatch prints a field-by-field diff.

The drift it found

A fold put the folded group's rail last in the list's rail records (foldLines), and showMissing (#23182's path) appended the rail it showed. The rails drew in the right place, but in a different order from a rebuild: 12 mismatches over 6 of the 7 operations, all this one cause. Both now insert a rail at its place in the list, top to bottom.

Changelog

none

No capture: nothing users see changes. The fix only reorders the list's internal rail records (groupLineViews.lines); every rail already drew at the same frame.

Observed / Not run

  • Red (test only, 4d32197 plus the base compile fix): package-test-lane.sh ... SidebarIncrementalMatchesRebuildTests, fleet step 98ba6d7e5236: 12 issues, every one a rail-order diff, e.g. order: [g-infra, g-work] vs fresh: [g-work, g-infra] after Work folds and opens.
  • Green (test and fix, 3de49be plus the same base fix): same command, fleet step 8b99271d3821: passes.
  • This head merges feat-cmux-next at 0284c98 (with cmux-next tests: RemoteChildSessionsTests' fake client states isReadOnly (base red: tests don't compile) #23362, the base compile fix); prepush-gate clean. Not run on this merged head yet; CI runs it.
  • Review: priority-view PASS on 81a8496 (comment 6315717000). Its note on tabAcrossGroups (the step passes on an unexpected drop) is a follow-up.
  • Not covered: Group by Folder, filters, Pinned and custom sections, multi-row drags, and drags with motion on (mid-flight frames).

🤖 Generated with Claude Code

Agent session: cosmic-gray

teamleaderleo and others added 3 commits October 21, 2026 04:34
…n list order

foldLines appended the folded group's rail and showMissing appended a shown one, so after a fold
the list's rails were out of the order a full rebuild gives them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15ba7-3152-78b2-bfdf-bb6eb0d5a053&host=cosmic-gray
@teamleaderleo teamleaderleo added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 21, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by claude-sr2/sidebar-drag-bugs on cosmic-gray · session

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 81a8496: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr23389 -f ref=cmux/sidebar-rebuild-invariant -f platform=macos -f backend_mode=local

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

CI passes on 81a8496efa (run 39598285143 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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

PASS (review on head 81a8496, correctness first; priority-view lane, session 6807b41c).

The test goes through the app's paths.

  • Folds go model.send(.toggleCollapse(.group)) → SidebarDocStore+Groups → model.onGroupFolded (SidebarListView.swift:154) → foldGroup → applyFold → foldLines. This is the incremental path, not a reload. Its guards (drag/settling/landing == nil) hold because the test finishes each landing before it folds.
  • Drags go beginDrag / updateDrag / finishDrag with a real SidebarDocDrag, so the document's acceptance table and drop callback both run. The test also checks that doc.drops is not empty. The drop's settle reaches showMissing (SidebarListView+Drag.swift:318).
  • The fresh list is a separate SidebarView on a copied model, and the fold state travels in the sections. The red/green pair (98ba6d7e5236 → 8b99271d3821) shows the fold path ran.

The fix is correct.

  • In foldLines, the lines below the header are shifted by dy first. They stay below header.y, so inserting own at the first line with minY > header.y puts it between the lines above and below the group.
  • showMissing inserts by minY, and a tie goes after, which is fine.

Non-blocking notes

  1. Only the rail lines field catches this: the drawn rails are sorted before the compare. Today the order of groupLineViews.lines has no visible effect; its only reader is foldLines, which filters it by group. So the fix keeps the internal records consistent and prevents later drift; it does not fix anything users see. The PR body says this correctly. The cubic summary ("drawn rails were in a different order") overstates it.
  2. In tabAcrossGroups, default: break together with #expect(drop != nil) passes even when the drop lands somewhere unexpected, and then the echo is skipped. Asserting the expected case, into Work between docs and web, would make that step bite.

I did not run it; I relied on your fleet red and green and on CI for the merged head.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-zb9fl created for this PR by the cmux coordinator.

@teamleaderleo
teamleaderleo merged commit 2ef423b into feat-cmux-next Oct 21, 2026
@teamleaderleo
teamleaderleo deleted the cmux/sidebar-rebuild-invariant branch October 21, 2026 12:22
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

teamleaderleo added a commit that referenced this pull request Oct 21, 2026
…, Pinned and custom sections, multi-row drags and motion; a dragged tab stays glass (#23542)

* test(next): sidebar rebuild invariant covers Group by Folder, filters, Pinned and custom sections, multi-row drags and motion; a dragged tab stays glass

Extends SidebarIncrementalMatchesRebuildTests (#23389) to the gaps it named,
each through the list's own pointer and key handlers, and pins #22807: the
tab drag lands in Work between docs and web, and a lifted strip tab keeps
its glass fill at every step of a real pointer drag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15ba7-3152-78b2-bfdf-bb6eb0d5a053&host=cosmic-gray

* test(next): the rebuild check waits for the list's springs with motion on; the gap drags move like a hand

With motion on the check compared after 90 ms, while springs and fades still
ran: half-point offsets and rows mid-fade. It now waits until no animation
runs in the list (at most 3 s), then compares. The gap scenarios' drags move
4 pt per step, and the Pinned and two-row drags check what they are about.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15ba7-3152-78b2-bfdf-bb6eb0d5a053&host=cosmic-gray

* test(next): the rebuild check waits until the list stops moving, not for layer animations

A row's spring writes its frame on each display frame and leaves no layer
animation, so the check now waits until two looks 50 ms apart agree (at
most 3 s) before it compares.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15ba7-3152-78b2-bfdf-bb6eb0d5a053&host=cosmic-gray

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant