Skip to content

fix: keep rapid document workspace placements ordered - #27431

Closed
teamleaderleo wants to merge 9 commits into
feat-cmux-nextfrom
lane/nx-sidebar-classic-order-race
Closed

teamleaderleo wants to merge 9 commits into
feat-cmux-nextfrom
lane/nx-sidebar-classic-order-race

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Rapid workspace creation can resolve successive .bottom placements against the sidebar projection from before the previous document move. The document mirror already applies each move synchronously; this fix refreshes the document-backed projection through the existing rows writer after each accepted placement. The next placement sees the preceding order without adding another optimistic owner.

The accepted document path from #25654 remains intact. Resync also preserves document-owned mixed order instead of re-enabling legacy ungrouped-first drop targeting.

Validation

Current head: de1e4c67e34f813e4c45b5a847d733333f556442, rebased on 0b45ce8ec311c0eb5148fe3e8887eda2d8e5b373, including merged parent #26153 and Classic fixture isolation #27421.

  • Independent whole-diff Codex review: gpt-6.1-sol high, SHIP at this exact head (base 0b45ce8e); no actionable P0–P2 findings. Sol is an authorized interchangeable Codex reviewer. Primary inspection covered the actual production and test diffs.
  • Native candidate step f5029adce85980c6aa99baa5: compile passed; PlacementDocumentWriteOrderTests completed 1/1, 0.333 s. The real Rust client/server mirrors commit and reload k1,k2,k3,k4; the three moves anchor after k1,k2,k3.
  • Native base-production regression step 8ea5fc6d352e9265ad094f0d at baseline 151eb767: compile passed and completed with four intended ordering assertions. Client, visible and freshly reloaded rows were k1,k4,k3,k2; all three operations anchored after k1.
  • Before capture step 5a0506c0845e11411ad55aad, baseline 6d53e3f6: the same four intended assertions, complete summary in 0.358 s, with actual native rows captured. This baseline adds only a capture-helper no-animation reload to settle its existing committed rows before taking the snapshot; production is unchanged from the base.
  • prepush-gate.py, git diff --check, hosted fast gates, defaults, Release compile, capture and risk evidence pass at the current head.
  • The single ten-pass Classic run is still active at this head on top of the fixture-isolation merge (00f6e1f4f33858685b1e58c9). Its partial results are not a ten-pass acceptance claim. The existing resumed-import case aRunCutOffMidwayFinishesWithoutDuplicates is explicitly marked a known ordering issue on base and remains separately tracked.

Performance

Metric Before After Delta Harness
Incorrect anchors in a three-placement document burst 2 0 -2 Real SidebarMirror + held remote; base-production 8ea5fc6d / exact-head f5029adc

These counts prove the stale-anchor case only. They do not establish zero ordering failures across all Classic simulations.

Overlap

The parent #26153 has merged and this PR targets feat-cmux-next. The placement path overlaps #21914, #25654 and #23071; either open placement change can rebase on the other. The accepted SidebarDocMachines.place and synchronous document reshow from #25654 are preserved.

Takes back #25654: deliberately extends its accepted document-placement return path to refresh bridge rows synchronously before the next same-turn placement. It changes when the normal rows reflect the accepted move.

Risk

Projection refresh is O(rows) per document-backed placement and runs only on this existing placement path. It uses SidebarDocStore.show and SidebarBridge.resync, preserving the single rows writer and existing refusal handling. No timer, observer or animation loop is added. Parent and isolation dependencies have merged; the shared Swift and sidebar dogfood holds are lifted.

Captures

After at the exact head: first, second, third, fourth. These are actual native AppKit fixture captures using scripted transport and real Rust client/server mirrors. The transparent sidebar background is preserved.

Light Dark
after light after dark

Before with current-base production: first, fourth, third, second. The fixture-only baseline 6d53e3f6 settles native views to their actual committed model before recording; it does not change the order. Both stills passed the shared private-text check.

Light Dark
before light before dark

Agent session: agent session

@teamleaderleo

teamleaderleo commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · took over from an earlier session · session

Base automatically changed from lane/nx-sidebar-rapid-bottom-placement to feat-cmux-next October 30, 2026 01:27
@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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

Review d1050c1: 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=pr27431 -f ref=lane/nx-sidebar-classic-order-race -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on ea12ea6 to compare; do not merge until green; see lane-rules

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)/
test_every_listed_hook_is_one_glaeda_released_with_the_check (main.SigningHooksInSt
epWithGlaedaTests.test_every_listed_hook_is_one_glaeda_released_with_the_check)
View Logs

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at de1e4c6

No blocking findings.

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmux-bot approves de1e4c6: an independent review passed this exact head (next-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Closing: superseded by #28219, a carry of this PR's commits with current feat-cmux-next merged in to clear the fork-point Script tests red. Review and landing continue there.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex-review-box/gpt-6-astra high, lane-place): SHIP at d1050c1

SHIP

  1. No findings.

teamleaderleo added a commit that referenced this pull request Oct 30, 2026
…8219)

* test: expose document-backed bottom placement burst ordering

* fix: refresh document-backed rows between workspace placements

* test: keep placement remote fixture compileable

* test: observe held placement writes without a scheduling race

* test: capture committed workspace document order

Sidebar-Look: no visual change; opt-in native regression captures only

* test: preserve document mixed order during placement resync

* fix: retain document mixed order during sidebar resync

* test: isolate the document placement regression with live app suites

* fix: derive sidebar grouping from active document

* test: use fake python for placement document fixture

Agent-Session: https://cmux.com/deeplink/agent?id=aid-4dd8b574224f6367f4ce

* test: cover inactive document with cached drag

Agent-Session: https://cmux.com/deeplink/agent?id=aid-4dd8b574224f6367f4ce

* test: verify legacy drop target after document disconnect

Agent-Session: https://cmux.com/deeplink/agent?id=aid-4dd8b574224f6367f4ce

* fix: reshow document after local placement

Agent-Session: https://cmux.com/deeplink/agent?id=aid-4dd8b574224f6367f4ce
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Closed earlier as an orphan; its fix landed through the carry #28219 as f287294: the sidebar resync now derives SidebarDocWiring.isActive, with a red/green PlacementDocumentWriteOrderTests (fails without the fix at :198/:215, passes with it).

This branch was previously deployed

1 inactive deployment
ci-routing-agent — d1050c19 Deployed Oct 30, 2026 by teamleaderleo via namespace autofix #7594
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