Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,7 @@ struct ContentView: View {
}

private var sidebarView: some View {
VerticalTabsSidebar(
let sidebar = VerticalTabsSidebar(
updateViewModel: updateViewModel,
fileExplorerState: fileExplorerState,
windowId: windowId,
Expand All @@ -1671,6 +1671,17 @@ struct ContentView: View {
selection: $sidebarSelectionState.selection,
selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, sidebarRenderWorkerClient: $sidebarRenderWorkerClient
)
return Group {
if CmuxFeatureFlags.shared.isAppKitSidebarListEnabled {
// FLAG(sidebar-appkit-list-experiment): parent-driven
// re-evaluations (divider width ticks, unrelated ContentView
// state churn) skip the sidebar subtree; all sidebar content
// flows through tracked dependencies that bypass the gate.
sidebar.equatable()
} else {
sidebar
}
}
.frame(width: sidebarWidth)
.frame(maxHeight: .infinity, alignment: .topLeading)
}
Expand Down Expand Up @@ -9933,7 +9944,20 @@ extension SidebarDragState {
/// so pressing/releasing the modifier key while the menu is up does not flip
/// the underlying row's shortcut badges (which would be visible around the
/// open context menu). All other rows transition live.
struct VerticalTabsSidebar: View {
struct VerticalTabsSidebar: View, Equatable {
// Equatable gates only parent-driven re-evaluation: closures and
// Bindings are excluded on purpose (recreated per parent eval but
// functionally identical), and every data source the body renders from
// (@EnvironmentObject, @ObservedObject, @Binding, @State) invalidates
// this view directly, bypassing the gate. See TabItemView for the
// precedent.
static func == (lhs: VerticalTabsSidebar, rhs: VerticalTabsSidebar) -> Bool {
lhs.windowId == rhs.windowId
&& lhs.observedWindow === rhs.observedWindow
&& lhs.updateViewModel === rhs.updateViewModel
&& lhs.fileExplorerState === rhs.fileExplorerState
}

var updateViewModel: UpdateStateModel
@ObservedObject var fileExplorerState: FileExplorerState
let windowId: UUID
Expand Down Expand Up @@ -10854,6 +10878,11 @@ struct VerticalTabsSidebar: View {
private func appKitWorkspaceTableRows(
renderContext: WorkspaceListRenderContext
) -> [SidebarWorkspaceTableRowConfiguration] {
#if DEBUG
// One line per full row-projection rebuild: the countable signal for
// whether a change class re-renders the sidebar subtree or skips it.
cmuxDebugLog("sidebar.table.rowsBuild items=\(renderContext.workspaceRenderItems.count)")
#endif
let unreadSummariesByWorkspaceId = sidebarUnread.summaryByWorkspaceId
let notificationIndex = SidebarWorkspaceNotificationIndex(
notifications: notificationStore.notifications
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,15 @@ final class SidebarGroupHeaderTableCellView: NSTableCellView {
plusButton.setRevealed(isPointerHovering && !contextMenuVisible && !showsHint)
}

/// Authoritative hover enforcement: the controller sweeps visible cells
/// so hover-revealed chrome cannot strand on rows the pointer left
/// (row-index/id races during churn made per-transition repaints miss).
func enforcePointerHovering(_ hovering: Bool) {
guard isPointerHovering != hovering else { return }
isPointerHovering = hovering
updatePlusVisibility()
}

// MARK: Layout

/// Deterministic row height; must stay in lockstep with `layout()`.
Expand Down
17 changes: 17 additions & 0 deletions Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowCellView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,23 @@ final class SidebarWorkspaceRowTableCellView: NSTableCellView {
closeButton.setRevealed(showsCloseNow)
}

/// Authoritative hover enforcement: the controller sweeps visible cells
/// so hover-revealed chrome cannot strand on rows the pointer left
/// (row-index/id races during churn made per-transition repaints miss).
func enforcePointerHovering(_ hovering: Bool) {
guard isPointerHovering != hovering else { return }
isPointerHovering = hovering
// Full re-apply: hover gates more than the close button (the
// trailing badge and spinner hide while the close button shows), and
// re-deriving that subset here would drift from applyModel.
if let model {
applyModel(model)
needsLayout = true
} else {
updateCloseVisibility()
}
}

private func configureMetadata(model: SidebarWorkspaceRowModel, palette: SidebarRowPalette) {
let entries = model.snapshot.metadataEntries
let visible = model.settings.visibleAuxiliaryDetails.showsMetadata ? Array(entries.prefix(3)) : []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,11 @@ struct SidebarWorkspaceRowCommands {

// MARK: Selection (parity with TabItemView.updateSelection)

func updateSelection() {
func updateSelection(modifiers: NSEvent.ModifierFlags = NSEvent.modifierFlags) {
#if DEBUG
cmuxDebugLog("sidebar.select.enter workspace=\(tab.id.uuidString.prefix(5)) hasTabManager=\(tabManager != nil)")
#endif
guard let tabManager else { return }
let modifiers = NSEvent.modifierFlags
let isCommand = modifiers.contains(.command)
let isShift = modifiers.contains(.shift)
let wasSelected = tabManager.selectedTabId == tab.id
Expand Down
56 changes: 56 additions & 0 deletions Sources/Sidebar/AppKitList/SidebarSelectionCoalescer.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import AppKit
import QuartzCore

/// Coalesces rapid plain-click workspace selections to the latest request.
///
/// A selection commit re-renders the container and swaps the terminal
/// content (~tens of ms); without coalescing, a burst of clicks queues one
/// full commit per click and later selections feel progressively slower.
/// Leading edge applies immediately (single clicks keep their latency);
/// clicks landing inside the window replace the pending request and one
/// trailing fire applies only the newest. The row's optimistic press
/// highlight still tracks every click instantly.
@MainActor
final class SidebarSelectionCoalescer {
private var pendingApply: (() -> Void)?
private var trailingTask: Task<Void, Never>?
private var lastApplied: CFTimeInterval = 0
private let window: TimeInterval
private let clock: any Clock<Duration>

init(window: TimeInterval = 0.1, clock: any Clock<Duration> = ContinuousClock()) {
self.window = window
self.clock = clock
}

func request(_ apply: @escaping @MainActor () -> Void) {
let now = CACurrentMediaTime()
if trailingTask == nil, now - lastApplied >= window {
lastApplied = now
apply()
return
}
pendingApply = apply
guard trailingTask == nil else { return }
let delay = max(0, window - (now - lastApplied))
// Injected-Clock sleep with cancellation wired to `cancel()`, per the
// bounded-delay policy (no raw Task.sleep in production paths).
trailingTask = Task { [weak self, clock] in
try? await clock.sleep(for: .seconds(delay))
guard let self, !Task.isCancelled else { return }
self.trailingTask = nil
self.lastApplied = CACurrentMediaTime()
let apply = self.pendingApply
self.pendingApply = nil
apply?()
}
}

/// Drops any pending request. Used before selection paths that must not
/// be reordered (modifier clicks mutate the multi-selection set).
func cancel() {
trailingTask?.cancel()
trailingTask = nil
pendingApply = nil
}
}
Comment on lines +14 to +56

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Timing repair leaves the commit cost unaddressed

The cmux-swift-architectural-rethink rule flags timing repair paths used to paper over rendering races. The coalescer reduces how often updateSelection triggers a container re-render and terminal-content swap, but does not name or fix why a single updateSelection call is expensive enough to cause noticeable serial slowdown. A rapid burst on a slower machine, or a future caller outside the flag, will surface the same problem. Consider documenting what the expensive operation is and whether it can be made idempotent (skipping work when the workspace being selected is already selected) or deferred to the next run-loop turn.

40 changes: 39 additions & 1 deletion Sources/Sidebar/AppKitList/SidebarWorkspaceTableController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
}
synchronizeAppKitDropIndicator(actions: actions)
recomputeHoveredRow()
enforceHoverOnVisibleCells()
updateDropTargets()
}

Expand All @@ -171,7 +172,19 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
#endif
guard rows.indices.contains(row) else { return }
if let actions = rows[row].appKitWorkspaceRowActions {
actions.commands.updateSelection()
// Capture modifiers at click time: a coalesced (trailing) apply
// must not re-read the keyboard ~100ms later.
let modifiers = NSEvent.modifierFlags
if modifiers.contains(.command) || modifiers.contains(.shift) {
// Multi-select mutations are order-dependent; apply in order,
// never dropping intermediates.
selectionCoalescer.cancel()
actions.commands.updateSelection(modifiers: modifiers)
} else {
selectionCoalescer.request {
actions.commands.updateSelection(modifiers: modifiers)
}
}
} else if let headerActions = rows[row].appKitGroupHeaderActions {
headerActions.onFocusAnchor()
}
Expand Down Expand Up @@ -363,9 +376,11 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
scheduleWidthRemeasure()
}
recomputeHoveredRow()
enforceHoverOnVisibleCells()
updateDropTargets()
}

private let selectionCoalescer = SidebarSelectionCoalescer()
private var lastMeasuredWidth: CGFloat = 0
private var widthRemeasureTask: Task<Void, Never>?

Expand Down Expand Up @@ -416,6 +431,29 @@ final class SidebarWorkspaceTableController: NSObject, NSTableViewDataSource, NS
reconfigureVisibleRows(indexes)
}

/// Authoritative pass over visible cells so hover-revealed chrome (close
/// button, header plus) cannot strand: per-transition repaints resolve
/// ids against a rows array that can mutate in the same tick (content
/// churn scrolling rows under a parked pointer), and a missed repaint
/// left multiple rows showing hover chrome at once.
private func enforceHoverOnVisibleCells() {
guard let table = containerView?.tableView else { return }
let visible = table.rows(in: table.visibleRect)
for row in visible.lowerBound..<(visible.lowerBound + visible.length)
where rows.indices.contains(row) {
let rowId = rows[row].id
let hovering = hoveredRowId == rowId && contextMenuRowId != rowId
switch table.view(atColumn: 0, row: row, makeIfNecessary: false) {
case let cell as SidebarGroupHeaderTableCellView:
cell.enforcePointerHovering(hovering)
case let cell as SidebarWorkspaceRowTableCellView:
cell.enforcePointerHovering(hovering)
default:
break
}
}
}

private func reconfigureVisibleRows(_ indexes: IndexSet) {
guard let table = containerView?.tableView else { return }
for row in indexes where rows.indices.contains(row) {
Expand Down
4 changes: 4 additions & 0 deletions cmux.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1505,6 +1505,7 @@ C0DE71B10000000000000001 /* AppDelegate+AgentChatNotifications.swift in Sources
AABBCC00000000000000020A /* SidebarRowDragGate.swift in Sources */ = {isa = PBXBuildFile; fileRef = AABBCC000000000000000209 /* SidebarRowDragGate.swift */; };
C0DE35010000000000000001 /* SidebarScrim.swift in Sources */ = {isa = PBXBuildFile; fileRef = C0DE35010000000000000002 /* SidebarScrim.swift */; };
C9A57513C9A57513C9A57513 /* SidebarScrollViewConfiguratorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9A57514C9A57514C9A57514 /* SidebarScrollViewConfiguratorTests.swift */; };
B804A0280000000000000028 /* SidebarSelectionCoalescer.swift in Sources */ = {isa = PBXBuildFile; fileRef = B804B0280000000000000028 /* SidebarSelectionCoalescer.swift */; };
E62155868BB29FEB5DAAAF25 /* SidebarSelectionState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9AD52285508B1D6A9875E7B3 /* SidebarSelectionState.swift */; };
F57072635F25EBCA741E125D /* SidebarState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1614EAD3CCF70A177A51BD1 /* SidebarState.swift */; };
D7344F010000000000000001 /* SidebarTabDragPayload.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7344F010000000000000002 /* SidebarTabDragPayload.swift */; };
Expand Down Expand Up @@ -3526,6 +3527,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
AABBCC000000000000000209 /* SidebarRowDragGate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarRowDragGate.swift; sourceTree = "<group>"; };
C0DE35010000000000000002 /* SidebarScrim.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarScrim.swift; sourceTree = "<group>"; };
C9A57514C9A57514C9A57514 /* SidebarScrollViewConfiguratorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarScrollViewConfiguratorTests.swift; sourceTree = "<group>"; };
B804B0280000000000000028 /* SidebarSelectionCoalescer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/AppKitList/SidebarSelectionCoalescer.swift; sourceTree = "<group>"; };
9AD52285508B1D6A9875E7B3 /* SidebarSelectionState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarSelectionState.swift; sourceTree = "<group>"; };
D1614EAD3CCF70A177A51BD1 /* SidebarState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarState.swift; sourceTree = "<group>"; };
D7344F010000000000000002 /* SidebarTabDragPayload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Sidebar/SidebarTabDragPayload.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4575,6 +4577,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
B804B0250000000000000025 /* SidebarWorkspaceRowSlotViews.swift */,
B804B0260000000000000026 /* SidebarWorkspaceRowSupportViews.swift */,
B804B0270000000000000027 /* SidebarDividerTrackingView.swift */,
B804B0280000000000000028 /* SidebarSelectionCoalescer.swift */,
B804B0210000000000000021 /* SidebarGroupHeaderRowView.swift */,
C9A57302C9A57302C9A57302 /* SidebarWorkspaceGroupHeaderMetrics.swift */,
C9A5710AC9A5710AC9A5710A /* SidebarWorkspaceGroupingMetrics.swift */,
Expand Down Expand Up @@ -7547,6 +7550,7 @@ C0DE71B10000000000000002 /* AppDelegate+AgentChatNotifications.swift */ = {isa =
AABBCC000000000000000208 /* SidebarRowAccessibilityModifier.swift in Sources */,
AABBCC00000000000000020A /* SidebarRowDragGate.swift in Sources */,
C0DE35010000000000000001 /* SidebarScrim.swift in Sources */,
B804A0280000000000000028 /* SidebarSelectionCoalescer.swift in Sources */,
E62155868BB29FEB5DAAAF25 /* SidebarSelectionState.swift in Sources */,
F57072635F25EBCA741E125D /* SidebarState.swift in Sources */,
D7344F010000000000000001 /* SidebarTabDragPayload.swift in Sources */,
Expand Down