Skip to content

Sidebar: request an authoritative apply when a deferred row click parks - #9691

Merged
azooz2003-bit merged 2 commits into
mainfrom
issue-9690-sidebar-deferred-click-wakeup
Aug 6, 2026
Merged

azooz2003-bit merged 2 commits into
mainfrom
issue-9690-sidebar-deferred-click-wakeup

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Aug 6, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #9690.

Infrequently, sidebar row taps did nothing until an app deactivate/reactivate cycle. #9225 made a click that lands on a presentation-snapshot row (live action captures released) survive as deferredRowClick, replayed from the next authoritative apply. The replay was passive: applies only happen when the Equatable-gated VerticalTabsSidebar body re-evaluates, and the park itself mutates no SwiftUI-tracked state, so an idle app never re-armed the rows. The parked click waited on unrelated invalidation, which a focus cycle eventually provided via window-key row repaints.

The controller now fires onDeferredRowClickAwaitingApply when it parks a click. SidebarWorkspaceTableView forwards it to VerticalTabsSidebar, which bumps a @State token read by appKitWorkspaceScrollArea (the appKitPostResizeRefreshToken pattern), so the body re-evaluates, updateNSView re-applies fresh action-carrying rows, and the parked click replays immediately. The request fires only from a physical click, never from a replay re-park, so one request per click is the ceiling and it cannot loop; a fresh apply always carries live actions (appKitRowSnapshotCache caches display snapshots only).

Two commits: the first adds the regression test against an inert seam (red), the second wires the seam (green). The test drives the exact reveal gap (suspend, reveal, no apply), sends the row click, asserts the apply request fires once, then answers it like production and asserts the parked selection lands.

Localization audit: no user-facing strings added or changed.

🤖 Generated with Claude Code


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


Summary by cubic

Fixes sidebar row clicks that sometimes did nothing until app refocus by requesting an authoritative apply when a deferred click parks, so the click replays immediately. Fixes #9690.

  • Bug Fixes
    • Added onDeferredRowClickAwaitingApply in SidebarWorkspaceTableController, fired when a click is deferred on snapshot rows.
    • Forwarded via SidebarWorkspaceTableView to VerticalTabsSidebar, which bumps appKitTableApplyRequestToken to re-evaluate and re-apply rows.
    • Fires only for physical clicks (no loops); at most one request per click.
    • Added test to assert a single apply request and that the parked selection lands; no user-facing strings changed.

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

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved sidebar row selection when presentation reactivation is pending.
    • Deferred clicks are now applied reliably after the interface becomes available.
    • Prevented duplicate click handling and inconsistent selection updates.
  • Tests

    • Added regression coverage for suspended and reactivated sidebar interactions.

azooz2003-bit and others added 2 commits August 5, 2026 21:27
A click landing while row actions are detached is deferred (#9225) but the
replay only runs from the next authoritative apply, and nothing requests
one: the park mutates no SwiftUI-tracked state, the sidebar body is
Equatable-gated, and an idle app never re-arms the rows. The click stays
parked until unrelated invalidation, historically an app deactivate/
reactivate cycle.

The test drives the reveal gap (suspend + reveal without an apply), sends
the row click, and asserts the controller fires the new
onDeferredRowClickAwaitingApply seam exactly once; the seam is inert in
this commit so CI shows the test red.

Refs #9690

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tions

Fixes #9690: infrequently,
sidebar row taps did nothing until an app deactivate/reactivate cycle.

A click landing on a presentation-snapshot row (live action captures
released) is deferred and replayed from the next authoritative apply
(#9225). But the replay was passive: applies only happen when the
Equatable-gated sidebar body re-evaluates, and the park itself mutates no
SwiftUI-tracked state, so an idle app never re-armed the rows. The parked
click waited for unrelated invalidation, which an app focus cycle
eventually provided via window-key row repaints.

The controller now fires onDeferredRowClickAwaitingApply when it parks a
click; SidebarWorkspaceTableView forwards it to VerticalTabsSidebar, which
bumps a @State token read by appKitWorkspaceScrollArea (same pattern as
appKitPostResizeRefreshToken). The body re-evaluates, updateNSView
re-applies fresh action-carrying rows, and the parked click replays
immediately. The request fires only from a physical click, never from a
replay re-park, so a request per click is the ceiling and it cannot loop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e095805e-43da-4738-9325-42f12d2d5b54

📥 Commits

Reviewing files that changed from the base of the PR and between e012389 and df21c4d.

📒 Files selected for processing (4)
  • Sources/ContentView.swift
  • Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift
  • Sources/Sidebar/AppKitList/SidebarWorkspaceTableView.swift
  • cmuxTests/SidebarWorkspaceTableSuspensionTests.swift

📝 Walkthrough

Walkthrough

The sidebar now requests a SwiftUI reevaluation when a physical row click is deferred during presentation suspension. The next authoritative table apply replays the click. A regression test verifies one request and successful workspace selection.

Changes

Deferred sidebar click application

Layer / File(s) Summary
Deferred click notification
Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift, Sources/Sidebar/AppKitList/SidebarWorkspaceTableView.swift
The controller reports when a physical row click is parked awaiting actions. Replays do not issue another request. The table view forwards the callback to the coordinator.
Apply invalidation and replay coverage
Sources/ContentView.swift, cmuxTests/SidebarWorkspaceTableSuspensionTests.swift
ContentView increments an apply request token to trigger reevaluation. The test verifies one deferred-apply request and selection after the authoritative apply.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • manaflow-ai/cmux#9225: Introduced the deferred sidebar-click replay flow extended by this change.
  • manaflow-ai/cmux#9336: Modified deferred-click and selection-dispatch behavior in SidebarWorkspaceTableController.
  • manaflow-ai/cmux#9289: Also modified ContentView.swift and SidebarWorkspaceTableController.swift, but addressed unread-state propagation.

Suggested reviewers: austinywang, lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Cmux Swiftui State Layout ❓ Inconclusive I am still checking the repository rules and the new state callback path. Await the source and rule inspection before deciding.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: requesting an authoritative apply for deferred sidebar row clicks.
Description check ✅ Passed The description explains the bug, implementation, rationale, regression test, and localization impact, but omits explicit template sections for testing, demo video, and checklist.
Linked Issues check ✅ Passed The changes satisfy issue #9690 by actively requesting one authoritative apply for deferred physical clicks and replaying the parked selection.
Out of Scope Changes check ✅ Passed All code and test changes directly support the deferred sidebar row-click wake-up behavior described in issue #9690.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed The production diff adds only a SwiftUI state token and callback wiring; the AppKit controller is explicitly @MainActor, and no new models, Sendable references, protocols, or background UI access a...
Cmux Swift Blocking Runtime ✅ Passed The production diff adds only a synchronous callback and SwiftUI @State invalidation token; no new waits, sleeps, timers, polling, locks, or sync dispatch. Test awaits are test-only scaffolding.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only sidebar SwiftUI/AppKit table code and its regression test; it adds no browser.* socket command or WebKit wait routing change.
Cmux Expensive Synchronous Load ✅ Passed The full PR diff adds only sidebar state/callback plumbing and a test; it adds no RestorableAgentSessionIndex.load, agent-file parsing, directory scan, or per-record syscall on the main path.
Cmux Cache Substitution Correctness ✅ Passed The diff adds transient sidebar apply-request wiring only; it does not replace an authoritative read, and existing row snapshot caches remain display-only and outside persistence, history, or undo...
Cmux No Hacky Sleeps ✅ Passed The pull request changes only four Swift files; the rule covers non-Swift runtime code, and the patch adds no covered sleep, timer, polling, or fixed-delay construct.
Cmux Algorithmic Complexity ✅ Passed The production diff adds only scalar token state and callback forwarding; it introduces no collection scan, sort, filter, join, or nested loop. Existing row projection remains linear and unchanged.
Cmux Swift Concurrency ✅ Passed The diff adds a synchronous AppKit/SwiftUI invalidation callback and @State token; it adds no Dispatch queues, Combine state, completion-handler API, or fire-and-forget Task.
Cmux Swift @Concurrent ✅ Passed The PR adds only synchronous main-actor callback/state wiring; its new async test is @MainActor and only awaits a RunLoop.main UI flush, so no @concurrent violation exists.
Cmux Swift Package Boundaries ✅ Passed The diff adds only AppKit table-controller glue, an NSViewRepresentable callback, and SwiftUI state invalidation; it introduces no independent domain logic requiring a SwiftPM package.
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only Swift source and test files; it has no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes.
Cmux Swift Logging ✅ Passed The full PR adds only a #if DEBUG cmuxDebugLog with a numeric row index; cmuxDebugLog uses CMUXDebugLog.logDebugEvent, and no secrets or personal data are logged.
Cmux User-Facing Error Privacy ✅ Passed The production diff adds callback/state plumbing and a DEBUG-only diagnostic; it adds no user-facing errors, alerts, recovery copy, or sensitive implementation details.
Cmux Full Internationalization ✅ Passed The production diff adds only state, callback wiring, and a DEBUG-only log; it adds no user-facing text or localization/catalog/message changes.
Cmux Architecture Rethink ✅ Passed The diff adds a documented synchronous AppKit–SwiftUI callback bridge; ownership stays with the controller and SwiftUI token, with no timing, polling, lock, observer, or duplicate action path. The...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff only adds deferred-click callback and SwiftUI state-token wiring; it adds no standalone window, panel, controller, Window, or WindowGroup. Existing window references are unchanged.
Cmux Source Artifacts ✅ Passed The full PR diff contains only three hand-written Swift source files and one regression test; no logs, caches, screenshots, temp folders, build output, or other artifacts appear.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The new callback and @State token implement production wake-up behavior in ContentView and have production call sites; no test-only accessor or seam-named member was added. The DEBUG addition is lo...
Cmux No Ambient Global State ✅ Passed The production diff adds only instance state and injected callbacks on VerticalTabsSidebar, SidebarWorkspaceTableView, and SidebarWorkspaceTableController; it adds no ambient global state.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9690-sidebar-deferred-click-wakeup

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.

@azooz2003-bit
azooz2003-bit merged commit 86f8875 into main Aug 6, 2026
6 checks passed
@azooz2003-bit
azooz2003-bit deleted the issue-9690-sidebar-deferred-click-wakeup branch August 6, 2026 23:07
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.

Sidebar taps on a suspended presentation park forever: deferred row click has no wake-up, selection only applies after an app focus cycle

1 participant