From 45b2386ae91952cce029c404a5c37687bce77d93 Mon Sep 17 00:00:00 2001 From: Abdulaziz Albahar <67667005+azooz2003-bit@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:10:45 -0700 Subject: [PATCH 1/4] Add frame pacing to the workspace-list scroll probe and a timestamps live-update fixture mode The DEBUG scroll-metrics probe now records display-link frame pacing during its sweep (hitch frames, hitch ms/s, max frame ms) so workspace list scroll work is quantifiable before and after changes. The layout preview fixture gains CMUX_UITEST_WORKSPACE_LIST_PREVIEW_LIVE_UPDATES=timestamps, which restamps previewAt/lastActivityAt sub-minute without visible changes - the exact update shape the Mac emits while agents stream. Co-Authored-By: Claude Fable 5 --- .../WorkspaceListLayoutPreviewView.swift | 37 +++++++--- .../WorkspaceListScrollMetricsProbe.swift | 69 ++++++++++++++++++- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift index fe9b74ff0161..b8cc0dc3b0ea 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift @@ -10,18 +10,32 @@ import SwiftUI @MainActor @Observable private final class WorkspaceListLayoutPreviewModel { + /// The continuous update feed's payload shape + /// (`CMUX_UITEST_WORKSPACE_LIST_PREVIEW_LIVE_UPDATES`). + enum LiveUpdateMode { + /// No feed. + case off + /// `1`: visible churn — unread toggles plus activity restamps. + case visible + /// `timestamps`: sub-minute activity restamps only, the shape the Mac + /// emits while agents stream (`last_activity_at` is the latest + /// notification's `createdAt`). Rows render identically, so a correct + /// list does zero work per tick. + case timestampsOnly + } + var workspaces: [MobileWorkspacePreview] - private let liveUpdatesEnabled: Bool + private let liveUpdateMode: LiveUpdateMode /// Creates a preview model with an optional continuous update feed. - init(workspaces: [MobileWorkspacePreview], liveUpdatesEnabled: Bool) { + init(workspaces: [MobileWorkspacePreview], liveUpdateMode: LiveUpdateMode) { self.workspaces = workspaces - self.liveUpdatesEnabled = liveUpdatesEnabled + self.liveUpdateMode = liveUpdateMode } /// Mutates rotating row payloads until the view-owned task is cancelled. func runLiveUpdates() async { - guard liveUpdatesEnabled else { return } + guard liveUpdateMode != .off else { return } var updateLane = 0 while !Task.isCancelled { do { @@ -30,8 +44,11 @@ private final class WorkspaceListLayoutPreviewModel { return } for index in workspaces.indices where index % 10 == updateLane { - workspaces[index].hasUnread.toggle() + if liveUpdateMode == .visible { + workspaces[index].hasUnread.toggle() + } workspaces[index].previewAt = Date() + workspaces[index].lastActivityAt = Date() } updateLane = (updateLane + 1) % 10 } @@ -94,12 +111,16 @@ public struct WorkspaceListLayoutPreviewView: View { return workspace } : initialWorkspaces + let liveUpdateMode: WorkspaceListLayoutPreviewModel.LiveUpdateMode + switch environment["CMUX_UITEST_WORKSPACE_LIST_PREVIEW_LIVE_UPDATES"] { + case "1": liveUpdateMode = .visible + case "timestamps": liveUpdateMode = .timestampsOnly + default: liveUpdateMode = .off + } _model = State( initialValue: WorkspaceListLayoutPreviewModel( workspaces: fixtureWorkspaces, - liveUpdatesEnabled: environment[ - "CMUX_UITEST_WORKSPACE_LIST_PREVIEW_LIVE_UPDATES" - ] == "1" + liveUpdateMode: liveUpdateMode ) ) } diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift index 7815ba7d531e..5a06ffff43f0 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift @@ -64,6 +64,25 @@ final class WorkspaceListScrollMetricsProbeView: UIView { /// what the per-row estimate should have been. let realizedHeightCounts: [String: Int] let corrections: [Correction] + /// Display-link frame pacing sampled during the sweep; nil until the + /// sweep produced at least one frame interval. + let framePacing: FramePacing? + } + + /// Frame-pacing summary of the sweep, in the shape of MetricKit's hitch + /// metrics: a hitch frame is one whose interval overran the display's + /// expected interval by 50% or more, and hitch time is the overrun beyond + /// the expected interval summed across hitch frames. + private struct FramePacing: Encodable { + let frameCount: Int + let expectedFrameMs: Double + let averageFrameMs: Double + let maxFrameMs: Double + let hitchFrameCount: Int + let hitchTotalMs: Double + /// Hitch milliseconds per second of sweep (Apple's hitch-ratio unit; + /// <5 is "good", >10 is user-visible stutter). + let hitchMsPerSecond: Double } private static let logger = Logger(subsystem: "dev.cmux.ios", category: "scroll-metrics") @@ -81,6 +100,13 @@ final class WorkspaceListScrollMetricsProbeView: UIView { /// `contentSize.height` sampled once per sweep frame — the value the /// scroll indicator actually renders from at draw time. private var sampledDrawHeights: [Double] = [] + /// Interval between consecutive sweep display-link callbacks; a late + /// callback means the previous frame overran its budget (a hitch). + private var sweepFrameDurations: [Double] = [] + /// Per-callback expected frame interval (`targetTimestamp - timestamp`), + /// robust to ProMotion/simulator rate changes mid-sweep. + private var sweepExpectedDurations: [Double] = [] + private var lastSweepFrameTimestamp: CFTimeInterval? override func didMoveToWindow() { super.didMoveToWindow() @@ -97,7 +123,7 @@ final class WorkspaceListScrollMetricsProbeView: UIView { displayLink = link } - @objc private func tick() { + @objc private func tick(_ link: CADisplayLink) { switch phase { case .searching(let framesLeft): if let scrollView = Self.findListScrollView(in: window) { @@ -116,6 +142,11 @@ final class WorkspaceListScrollMetricsProbeView: UIView { } baselineAfterSettle() case .sweeping: + if let last = lastSweepFrameTimestamp { + sweepFrameDurations.append(link.timestamp - last) + sweepExpectedDurations.append(link.targetTimestamp - link.timestamp) + } + lastSweepFrameTimestamp = link.timestamp sampledDrawHeights.append(Double(listScrollView?.contentSize.height ?? 0)) stepSweep() case .finished: @@ -166,6 +197,11 @@ final class WorkspaceListScrollMetricsProbeView: UIView { Self.logger.notice( "scroll-metrics: sweep done corrections=\(self.corrections.count) totalAbsCorrection=\(self.totalAbsCorrection, format: .fixed(precision: 1)) initial=\(self.initialContentHeight, format: .fixed(precision: 1)) final=\(Double(self.listScrollView?.contentSize.height ?? 0), format: .fixed(precision: 1))" ) + if let pacing = framePacing { + Self.logger.notice( + "scroll-metrics: pacing frames=\(pacing.frameCount) hitchFrames=\(pacing.hitchFrameCount) hitchMs=\(pacing.hitchTotalMs, format: .fixed(precision: 1)) hitchMsPerS=\(pacing.hitchMsPerSecond, format: .fixed(precision: 2)) maxFrameMs=\(pacing.maxFrameMs, format: .fixed(precision: 1))" + ) + } } } @@ -205,6 +241,34 @@ final class WorkspaceListScrollMetricsProbeView: UIView { corrections.reduce(0) { $0 + abs($1.newHeight - $1.oldHeight) } } + private var framePacing: FramePacing? { + guard !sweepFrameDurations.isEmpty, !sweepExpectedDurations.isEmpty else { return nil } + let expected = sweepExpectedDurations.sorted()[sweepExpectedDurations.count / 2] + guard expected > 0 else { return nil } + let hitchThreshold = expected * 1.5 + var hitchFrameCount = 0 + var hitchTotal: Double = 0 + var maxDuration: Double = 0 + var total: Double = 0 + for duration in sweepFrameDurations { + total += duration + maxDuration = max(maxDuration, duration) + if duration >= hitchThreshold { + hitchFrameCount += 1 + hitchTotal += duration - expected + } + } + return FramePacing( + frameCount: sweepFrameDurations.count, + expectedFrameMs: expected * 1000, + averageFrameMs: total / Double(sweepFrameDurations.count) * 1000, + maxFrameMs: maxDuration * 1000, + hitchFrameCount: hitchFrameCount, + hitchTotalMs: hitchTotal * 1000, + hitchMsPerSecond: total > 0 ? (hitchTotal * 1000) / total : 0 + ) + } + private func writeReport() { guard let documents = FileManager.default.urls( @@ -223,7 +287,8 @@ final class WorkspaceListScrollMetricsProbeView: UIView { // written from a KVO callback inside a layout pass, where an extra // whole-content layout query is both wasted work and reentrant. realizedHeightCounts: finished ? realizedHeightCounts() : [:], - corrections: corrections + corrections: corrections, + framePacing: framePacing ) let encoder = JSONEncoder() encoder.outputFormatting = [.prettyPrinted, .sortedKeys] From b2a61802db2fb5fe7c95561d6d8f3354e2cbeb6b Mon Sep 17 00:00:00 2001 From: Abdulaziz Albahar <67667005+azooz2003-bit@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:10:45 -0700 Subject: [PATCH 2/4] Stop live workspace-list updates from re-running the diffable apply and re-rendering unchanged rows Two measured main-thread costs ran on every workspace-list emission while agents stream (the iOS workspace-list scroll stutter): 1. Any workspace field delta reconfigured the row, but the Mac restamps preview_at/last_activity_at from the latest notification on every emission while the row renders that time at minute granularity. Reconfigure now decides by render equivalence: full struct equality (fail-closed for future fields) with same-minute timestamps normalized out. 2. Payload-only updates rode NSDiffableDataSourceSnapshot.apply, which runs the diffable apply queue plus UITableView's whole batch-update pass per tick (~1.3ms on an M-series simulator, more on device) with nothing to diff. When no changed row's height key moved, the visible changed cells are now re-configured in place and offscreen rows pick up the payload on dequeue; height-changing payloads keep the snapshot path so UITableView re-queries heights. 30s fixture window, 400 rows, updates every 80ms, M-series simulator: timestamp-only churn 3.27s -> 2.59s CPU, visible churn 3.33s -> 3.00s; the diffable-apply subtree disappears from the sample profile. Sweep invariants hold: 0 contentSize corrections, 1 distinct draw height. Co-Authored-By: Claude Fable 5 --- .../WorkspaceListTableCoordinator.swift | 111 ++++++++++++++- .../WorkspaceListScrollUpdateTests.swift | 131 ++++++++++++++++++ 2 files changed, 238 insertions(+), 4 deletions(-) diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift index 4940abedfc22..47fa3571e6af 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift @@ -46,6 +46,27 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, private var heightCache = WorkspaceListRowHeightCache() private var configuredItemsByID: [String: WorkspaceListTableItem] + /// How one configuration update reached the table. + enum PayloadApplyRoute: Equatable { + /// No row renders differently; the table was not touched. + case noChange + /// Payload-only changes with stable heights; the visible changed + /// cells were re-configured in place, listed here by item id. + case reconfiguredInPlace([String]) + /// Structure or a row height changed; a snapshot was applied. + case snapshotApply + } + #if DEBUG + /// The most recent update's route (behavior hook for package tests). + private(set) var lastPayloadApplyRoute: PayloadApplyRoute? + #endif + + private func recordPayloadApplyRoute(_ route: PayloadApplyRoute) { + #if DEBUG + lastPayloadApplyRoute = route + #endif + } + init(configuration: WorkspaceListTable) { self.configuration = configuration self.configuredItemsByID = Dictionary( @@ -109,6 +130,7 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, let structureChanged = currentSnapshot.sectionIdentifiers != [Self.section] || currentSnapshot.itemIdentifiers != next.items var changed: [WorkspaceListTableItem] = [] + var changedRowHeightsStable = true if let previous { // This map already mirrors previousConfiguration. Reuse it instead // of rebuilding a second full index for every live row update. @@ -121,6 +143,11 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, next: next ) { changed.append(item) + if !structureChanged, changedRowHeightsStable, + heightCacheKey(for: oldItem, tableView: tableView, configuration: previous) + != heightCacheKey(for: item, tableView: tableView, configuration: next) { + changedRowHeightsStable = false + } } } } @@ -137,7 +164,30 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, } previousConfiguration = next - guard structureChanged || !changed.isEmpty else { return } + guard structureChanged || !changed.isEmpty else { + recordPayloadApplyRoute(.noChange) + return + } + + if !structureChanged, changedRowHeightsStable { + // Payload-only update: no row identity moved and no row height + // changed, so the snapshot has nothing to diff. Routing this + // through apply(_:animatingDifferences:) would still run the + // diffable apply queue plus UITableView's whole batch-update + // pass on every live preview/unread/chip tick while agents + // stream. Re-configure the visible changed cells in place (the + // exact work reconfigure performs); offscreen rows pick up the + // new payload from `configuredItemsByID` when they dequeue. + for item in changed { + guard + let indexPath = dataSource.indexPath(for: item), + let cell = tableView.cellForRow(at: indexPath) + else { continue } + configure(cell, for: configuredItemsByID[item.id] ?? item) + } + recordPayloadApplyRoute(.reconfiguredInPlace(changed.map(\.id))) + return + } var snapshot: NSDiffableDataSourceSnapshot if structureChanged { @@ -149,6 +199,7 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, } snapshot.reconfigureItems(changed) dataSource.apply(snapshot, animatingDifferences: false) + recordPayloadApplyRoute(.snapshotApply) } func tableView( @@ -610,12 +661,22 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, private func heightCacheKey( for item: WorkspaceListTableItem, tableView: UITableView + ) -> HeightCacheKey { + heightCacheKey(for: item, tableView: tableView, configuration: configuration) + } + + private func heightCacheKey( + for item: WorkspaceListTableItem, + tableView: UITableView, + configuration: WorkspaceListTable ) -> HeightCacheKey { let scale = tableView.window?.screen.scale ?? UIScreen.main.scale let kind: HeightKind switch item { case .workspace(let id, _): - let changesChipIdentity = workspaceChangesChipHeightIdentity(id: id) + let changesChipIdentity = workspaceChangesChipHeightIdentity( + id: id, configuration: configuration + ) if configuration.wrapWorkspaceTitles, let workspace = configuration.workspacesByID[id] { kind = .workspaceWrapped( @@ -673,7 +734,8 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, /// Separates chip modes and bounded digit-count widths that may wrap. private func workspaceChangesChipHeightIdentity( - id: MobileWorkspacePreview.ID + id: MobileWorkspacePreview.ID, + configuration: WorkspaceListTable ) -> WorkspaceChangesChipHeightKey? { guard configuration.workspaceChangesCapable, let workspace = configuration.workspacesByID[id], @@ -721,7 +783,9 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, previous.workspacesByID[id]?.macConnectionStatus ?? previous.connectionStatus let nextConnectionStatus = next.workspacesByID[id]?.macConnectionStatus ?? next.connectionStatus - return previous.workspacesByID[id] != next.workspacesByID[id] + return !Self.workspaceRenderEquivalent( + previous.workspacesByID[id], next.workspacesByID[id] + ) || workspaceChangesChipChanged(id: id, previous: previous, next: next) || oldItem.isIndentedWorkspace != item.isIndentedWorkspace || wasSelected != isSelected @@ -767,6 +831,45 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, } } + /// Whether two snapshots of a workspace render identically in the row. + /// + /// Full struct equality decides — fail-closed for any field this list + /// does not special-case, including ones added later — except the + /// activity timestamps: the row renders them at minute granularity + /// (``MobileWorkspacePreview/activityTimestampLabel(referenceDate:calendar:)``), + /// while the Mac restamps `last_activity_at`/`preview_at` from the latest + /// notification on every list emission. Sub-minute restamps therefore + /// must not count as changes, or every agent-output notification + /// re-renders rows that look exactly the same (measured at ~9ms of + /// main-thread work per tick on an M-series simulator, worse on device — + /// the workspace-list scroll stutter). + static func workspaceRenderEquivalent( + _ previous: MobileWorkspacePreview?, + _ next: MobileWorkspacePreview? + ) -> Bool { + guard var normalizedPrevious = previous, let next else { + return previous == nil && next == nil + } + if Self.sameRenderedMinute(normalizedPrevious.previewAt, next.previewAt) { + normalizedPrevious.previewAt = next.previewAt + } + if Self.sameRenderedMinute(normalizedPrevious.lastActivityAt, next.lastActivityAt) { + normalizedPrevious.lastActivityAt = next.lastActivityAt + } + return normalizedPrevious == next + } + + /// Whether the row's timestamp label renders the same for both dates. + /// The label shows a wall-clock minute (or month/day), so two dates in + /// the same calendar minute are indistinguishable. `nil` transitions are + /// render-relevant (the label source can change) and stay unequal. + private static func sameRenderedMinute(_ lhs: Date?, _ rhs: Date?) -> Bool { + if lhs == rhs { return true } + guard let lhs, let rhs else { return false } + return Int(lhs.timeIntervalSinceReferenceDate / 60) + == Int(rhs.timeIntervalSinceReferenceDate / 60) + } + private func workspaceActionAvailabilityChanged( previous: WorkspaceListTable, next: WorkspaceListTable diff --git a/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift index 65792399ed8a..1c2f6d03a394 100644 --- a/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift +++ b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/WorkspaceListScrollUpdateTests.swift @@ -60,6 +60,106 @@ import UIKit #expect(replacementTable.numberOfRows(inSection: 0) == 2) } + @Test func subMinuteActivityRestampDoesNoTableWork() { + // Second 0 of an epoch minute, so +2s stays inside the same rendered + // minute. The Mac restamps preview_at/last_activity_at from the + // latest notification on every list emission; a sub-minute restamp + // renders identically and must not touch the table. + let base = Date(timeIntervalSinceReferenceDate: 790_000_020) + var workspace = preview(id: "workspace-1", activityAt: base) + let coordinator = WorkspaceListTableCoordinator( + configuration: configuration(workspaces: [workspace]) + ) + let tableView = makeTableView() + coordinator.attach(to: tableView) + + workspace.previewAt = base.addingTimeInterval(2) + workspace.lastActivityAt = base.addingTimeInterval(2) + coordinator.update(configuration: configuration(workspaces: [workspace]), in: tableView) + + #expect(coordinator.lastPayloadApplyRoute == .noChange) + } + + @Test func minuteCrossingActivityRestampReconfiguresInPlace() { + // One second before a minute boundary, so +2s changes the rendered + // timestamp label and the row must re-render — but heights are + // untouched, so no snapshot apply is needed. + let base = Date(timeIntervalSinceReferenceDate: 790_000_019) + var workspace = preview(id: "workspace-1", activityAt: base) + let coordinator = WorkspaceListTableCoordinator( + configuration: configuration(workspaces: [workspace]) + ) + let tableView = makeTableView() + coordinator.attach(to: tableView) + + workspace.lastActivityAt = base.addingTimeInterval(2) + coordinator.update(configuration: configuration(workspaces: [workspace]), in: tableView) + + #expect( + coordinator.lastPayloadApplyRoute == .reconfiguredInPlace(["workspace.workspace-1"]) + ) + } + + @Test func previewTextChangeReconfiguresInPlaceWithoutSnapshotApply() { + var workspace = preview(id: "workspace-1", activityAt: Date(timeIntervalSinceReferenceDate: 790_000_020)) + let coordinator = WorkspaceListTableCoordinator( + configuration: configuration(workspaces: [workspace]) + ) + let tableView = makeTableView() + coordinator.attach(to: tableView) + + workspace.previewText = "Agent finished: PR opened" + workspace.hasUnread = true + coordinator.update(configuration: configuration(workspaces: [workspace]), in: tableView) + + #expect( + coordinator.lastPayloadApplyRoute == .reconfiguredInPlace(["workspace.workspace-1"]) + ) + } + + @Test func descriptionArrivalChangesRowHeightThroughSnapshotApply() { + // A durable description adds a text line, changing the row's height + // key: this payload change must keep riding the snapshot apply so + // UITableView re-queries the row height. + var workspace = preview(id: "workspace-1", activityAt: Date(timeIntervalSinceReferenceDate: 790_000_020)) + let coordinator = WorkspaceListTableCoordinator( + configuration: configuration(workspaces: [workspace]) + ) + let tableView = makeTableView() + coordinator.attach(to: tableView) + + workspace.customDescription = "Durable workspace context" + coordinator.update(configuration: configuration(workspaces: [workspace]), in: tableView) + + #expect(coordinator.lastPayloadApplyRoute == .snapshotApply) + } + + @Test func workspaceRenderEquivalenceQuantizesOnlyTimestamps() { + let base = Date(timeIntervalSinceReferenceDate: 790_000_020) + var previous = preview(id: "workspace-1", activityAt: base) + var next = previous + + next.previewAt = base.addingTimeInterval(59) + next.lastActivityAt = base.addingTimeInterval(59) + #expect(WorkspaceListTableCoordinator.workspaceRenderEquivalent(previous, next)) + + next.lastActivityAt = base.addingTimeInterval(60) + #expect(!WorkspaceListTableCoordinator.workspaceRenderEquivalent(previous, next)) + + // nil transitions change the label source and stay render-relevant. + next = previous + next.lastActivityAt = nil + #expect(!WorkspaceListTableCoordinator.workspaceRenderEquivalent(previous, next)) + + // Any non-timestamp field still decides by full equality. + next = previous + next.hasUnread = true + #expect(!WorkspaceListTableCoordinator.workspaceRenderEquivalent(previous, next)) + #expect(WorkspaceListTableCoordinator.workspaceRenderEquivalent(nil, nil)) + previous.previewAt = nil + #expect(!WorkspaceListTableCoordinator.workspaceRenderEquivalent(previous, nil)) + } + @Test func coordinatorKeepsWorkspaceRowSwipeAndContextMenuActionsAvailable() { let capabilities = MobileWorkspaceActionCapabilities( supportsWorkspaceActions: true, @@ -115,6 +215,17 @@ import UIKit ) } + private func preview(id: String, activityAt: Date) -> MobileWorkspacePreview { + MobileWorkspacePreview( + id: .init(rawValue: id), + name: id, + previewText: "Build succeeded", + previewAt: activityAt, + lastActivityAt: activityAt, + terminals: [] + ) + } + private func configuration( workspaceIDs: [String], actionCapabilities: MobileWorkspaceActionCapabilities = .none, @@ -134,6 +245,26 @@ import UIKit workspace.actionCapabilities = actionCapabilities return workspace } + return configuration( + workspaces: workspaces, + requestWorkspaceClose: requestWorkspaceClose, + closeWorkspace: closeWorkspace, + setUnread: setUnread, + setPinned: setPinned, + renameRequest: renameRequest, + customizeRequest: customizeRequest + ) + } + + private func configuration( + workspaces: [MobileWorkspacePreview], + requestWorkspaceClose: ((MobileWorkspacePreview.ID) -> Void)? = nil, + closeWorkspace: ((MobileWorkspacePreview.ID) -> Void)? = nil, + setUnread: ((MobileWorkspacePreview.ID, Bool) -> Void)? = nil, + setPinned: ((MobileWorkspacePreview.ID, Bool) -> Void)? = nil, + renameRequest: ((MobileWorkspacePreview.ID) -> Void)? = nil, + customizeRequest: ((MobileWorkspacePreview.ID) -> Void)? = nil + ) -> WorkspaceListTable { return WorkspaceListTable( items: workspaces.map { .workspace($0.id, indented: false) }, workspacesByID: Dictionary(uniqueKeysWithValues: workspaces.map { ($0.id, $0) }), From 5363835ff7464039b00626968c37037a4371cf9f Mon Sep 17 00:00:00 2001 From: Abdulaziz Albahar <67667005+azooz2003-bit@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:43:18 -0700 Subject: [PATCH 3/4] Address review: isolate the DEBUG route probe, per-frame hitch budgets, in-bucket fixture restamps The apply-route test hook moves out of the production coordinator into a dedicated DEBUG file (extension + registry) with #if DEBUG call sites, so Release builds also stop allocating the changed-id array. The scroll probe judges each sweep frame against the expected interval captured at the callback that started it instead of the median, so a mid-sweep refresh-rate change is not misclassified as a hitch. The timestamps fixture mode restamps relative to each row's own clock so the first tick no longer jumps seeded hours-old timestamps across a rendered minute. Co-Authored-By: Claude Fable 5 --- .../WorkspaceListLayoutPreviewView.swift | 16 +++++- .../WorkspaceListScrollMetricsProbe.swift | 29 ++++++---- ...ListTableCoordinator+ApplyRouteProbe.swift | 53 +++++++++++++++++++ .../WorkspaceListTableCoordinator.swift | 27 +++------- 4 files changed, 91 insertions(+), 34 deletions(-) create mode 100644 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift index b8cc0dc3b0ea..008e66c33efb 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift @@ -46,9 +46,21 @@ private final class WorkspaceListLayoutPreviewModel { for index in workspaces.indices where index % 10 == updateLane { if liveUpdateMode == .visible { workspaces[index].hasUnread.toggle() + workspaces[index].previewAt = Date() + workspaces[index].lastActivityAt = Date() + } else { + // Restamp relative to the row's own clock: the seeded + // timestamps are hours old, so jumping them to `Date()` + // would change the rendered minute on every row's first + // tick and do real row work. A one-second bump keeps + // every tick a sub-minute, render-equivalent delta. + let restamped = (workspaces[index].lastActivityAt + ?? workspaces[index].previewAt + ?? Date()) + .addingTimeInterval(1) + workspaces[index].previewAt = restamped + workspaces[index].lastActivityAt = restamped } - workspaces[index].previewAt = Date() - workspaces[index].lastActivityAt = Date() } updateLane = (updateLane + 1) % 10 } diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift index 5a06ffff43f0..e9503dc9e99e 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListScrollMetricsProbe.swift @@ -103,10 +103,13 @@ final class WorkspaceListScrollMetricsProbeView: UIView { /// Interval between consecutive sweep display-link callbacks; a late /// callback means the previous frame overran its budget (a hitch). private var sweepFrameDurations: [Double] = [] - /// Per-callback expected frame interval (`targetTimestamp - timestamp`), - /// robust to ProMotion/simulator rate changes mid-sweep. + /// The expected interval for each entry of `sweepFrameDurations`, + /// captured as `targetTimestamp - timestamp` at the callback that STARTED + /// that frame, so each duration is judged against its own frame's budget + /// even when ProMotion/simulator refresh rates change mid-sweep. private var sweepExpectedDurations: [Double] = [] private var lastSweepFrameTimestamp: CFTimeInterval? + private var lastSweepExpectedDuration: Double? override func didMoveToWindow() { super.didMoveToWindow() @@ -142,11 +145,12 @@ final class WorkspaceListScrollMetricsProbeView: UIView { } baselineAfterSettle() case .sweeping: - if let last = lastSweepFrameTimestamp { + if let last = lastSweepFrameTimestamp, let expected = lastSweepExpectedDuration { sweepFrameDurations.append(link.timestamp - last) - sweepExpectedDurations.append(link.targetTimestamp - link.timestamp) + sweepExpectedDurations.append(expected) } lastSweepFrameTimestamp = link.timestamp + lastSweepExpectedDuration = link.targetTimestamp - link.timestamp sampledDrawHeights.append(Double(listScrollView?.contentSize.height ?? 0)) stepSweep() case .finished: @@ -242,25 +246,28 @@ final class WorkspaceListScrollMetricsProbeView: UIView { } private var framePacing: FramePacing? { - guard !sweepFrameDurations.isEmpty, !sweepExpectedDurations.isEmpty else { return nil } - let expected = sweepExpectedDurations.sorted()[sweepExpectedDurations.count / 2] - guard expected > 0 else { return nil } - let hitchThreshold = expected * 1.5 + guard !sweepFrameDurations.isEmpty, + sweepExpectedDurations.count == sweepFrameDurations.count else { return nil } + let medianExpected = sweepExpectedDurations.sorted()[sweepExpectedDurations.count / 2] + guard medianExpected > 0 else { return nil } var hitchFrameCount = 0 var hitchTotal: Double = 0 var maxDuration: Double = 0 var total: Double = 0 - for duration in sweepFrameDurations { + // Judge each frame against its own captured budget so a legitimate + // refresh-rate change mid-sweep is not misclassified as a hitch; the + // median is reported only as the representative expected interval. + for (duration, expected) in zip(sweepFrameDurations, sweepExpectedDurations) { total += duration maxDuration = max(maxDuration, duration) - if duration >= hitchThreshold { + if expected > 0, duration >= expected * 1.5 { hitchFrameCount += 1 hitchTotal += duration - expected } } return FramePacing( frameCount: sweepFrameDurations.count, - expectedFrameMs: expected * 1000, + expectedFrameMs: medianExpected * 1000, averageFrameMs: total / Double(sweepFrameDurations.count) * 1000, maxFrameMs: maxDuration * 1000, hitchFrameCount: hitchFrameCount, diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift new file mode 100644 index 000000000000..ec7fc9a9c0c1 --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift @@ -0,0 +1,53 @@ +#if os(iOS) && DEBUG +import Foundation + +/// DEBUG-only observation of which route a coordinator's most recent +/// configuration update took, isolated from the production type per the +/// source policy on test-only seams (mirrors how +/// `WorkspaceListScrollMetricsProbe` isolates DEBUG instrumentation). The +/// coordinator's `#if DEBUG` call sites record here; package tests read the +/// route back through ``WorkspaceListTableCoordinator/lastPayloadApplyRoute``. +extension WorkspaceListTableCoordinator { + /// How one configuration update reached the table. + enum PayloadApplyRoute: Equatable { + /// No row renders differently; the table was not touched. + case noChange + /// Payload-only changes with stable heights; the visible changed + /// cells were re-configured in place, listed here by item id. + case reconfiguredInPlace([String]) + /// Structure or a row height changed; a snapshot was applied. + case snapshotApply + } + + /// The most recent update's route, or nil before the first update. + var lastPayloadApplyRoute: PayloadApplyRoute? { + WorkspaceListApplyRouteProbe.lastRoute(for: self) + } + + func recordPayloadApplyRoute(_ route: PayloadApplyRoute) { + WorkspaceListApplyRouteProbe.record(route, for: self) + } +} + +/// Registry keyed by coordinator identity. Entries for deallocated +/// coordinators linger, which is acceptable for a DEBUG facility whose +/// population is bounded by the coordinators a test run creates. +@MainActor +private enum WorkspaceListApplyRouteProbe { + private static var lastRoutesByCoordinator: + [ObjectIdentifier: WorkspaceListTableCoordinator.PayloadApplyRoute] = [:] + + static func record( + _ route: WorkspaceListTableCoordinator.PayloadApplyRoute, + for coordinator: WorkspaceListTableCoordinator + ) { + lastRoutesByCoordinator[ObjectIdentifier(coordinator)] = route + } + + static func lastRoute( + for coordinator: WorkspaceListTableCoordinator + ) -> WorkspaceListTableCoordinator.PayloadApplyRoute? { + lastRoutesByCoordinator[ObjectIdentifier(coordinator)] + } +} +#endif diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift index 47fa3571e6af..49cb14401574 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator.swift @@ -46,27 +46,6 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, private var heightCache = WorkspaceListRowHeightCache() private var configuredItemsByID: [String: WorkspaceListTableItem] - /// How one configuration update reached the table. - enum PayloadApplyRoute: Equatable { - /// No row renders differently; the table was not touched. - case noChange - /// Payload-only changes with stable heights; the visible changed - /// cells were re-configured in place, listed here by item id. - case reconfiguredInPlace([String]) - /// Structure or a row height changed; a snapshot was applied. - case snapshotApply - } - #if DEBUG - /// The most recent update's route (behavior hook for package tests). - private(set) var lastPayloadApplyRoute: PayloadApplyRoute? - #endif - - private func recordPayloadApplyRoute(_ route: PayloadApplyRoute) { - #if DEBUG - lastPayloadApplyRoute = route - #endif - } - init(configuration: WorkspaceListTable) { self.configuration = configuration self.configuredItemsByID = Dictionary( @@ -165,7 +144,9 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, previousConfiguration = next guard structureChanged || !changed.isEmpty else { + #if DEBUG recordPayloadApplyRoute(.noChange) + #endif return } @@ -185,7 +166,9 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, else { continue } configure(cell, for: configuredItemsByID[item.id] ?? item) } + #if DEBUG recordPayloadApplyRoute(.reconfiguredInPlace(changed.map(\.id))) + #endif return } @@ -199,7 +182,9 @@ final class WorkspaceListTableCoordinator: NSObject, UITableViewDelegate, } snapshot.reconfigureItems(changed) dataSource.apply(snapshot, animatingDifferences: false) + #if DEBUG recordPayloadApplyRoute(.snapshotApply) + #endif } func tableView( From 5875bf67f93fcd6b4fd44e5cfb9ed9f2a313edd3 Mon Sep 17 00:00:00 2001 From: Abdulaziz Albahar <67667005+azooz2003-bit@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:55:31 -0700 Subject: [PATCH 4/4] Address review round 2: in-bucket fixture restamps, weak-key route probe storage The timestamps fixture bump now wraps to the start of the row's current minute instead of crossing into the next one, so every tick honors the mode's zero-work contract. The DEBUG route registry keys weakly through NSMapTable so entries die with their coordinator and a reused address cannot return a predecessor's route. Co-Authored-By: Claude Fable 5 --- .../WorkspaceListLayoutPreviewView.swift | 18 +++++++++++----- ...ListTableCoordinator+ApplyRouteProbe.swift | 21 ++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift index 008e66c33efb..519885f96628 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListLayoutPreviewView.swift @@ -52,12 +52,20 @@ private final class WorkspaceListLayoutPreviewModel { // Restamp relative to the row's own clock: the seeded // timestamps are hours old, so jumping them to `Date()` // would change the rendered minute on every row's first - // tick and do real row work. A one-second bump keeps - // every tick a sub-minute, render-equivalent delta. - let restamped = (workspaces[index].lastActivityAt + // tick and do real row work. The bump also wraps back to + // the start of the row's current minute rather than + // crossing into the next one, so EVERY tick is a + // render-equivalent delta (this mode's zero-work + // contract), not just the first fifty-nine. + let current = workspaces[index].lastActivityAt ?? workspaces[index].previewAt - ?? Date()) - .addingTimeInterval(1) + ?? Date() + let minute = (current.timeIntervalSinceReferenceDate / 60) + .rounded(.down) + var restamped = current.addingTimeInterval(1) + if (restamped.timeIntervalSinceReferenceDate / 60).rounded(.down) != minute { + restamped = Date(timeIntervalSinceReferenceDate: minute * 60) + } workspaces[index].previewAt = restamped workspaces[index].lastActivityAt = restamped } diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift index ec7fc9a9c0c1..65b56a51735b 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListTableCoordinator+ApplyRouteProbe.swift @@ -29,25 +29,32 @@ extension WorkspaceListTableCoordinator { } } -/// Registry keyed by coordinator identity. Entries for deallocated -/// coordinators linger, which is acceptable for a DEBUG facility whose -/// population is bounded by the coordinators a test run creates. +/// Registry with weak coordinator keys: an entry dies with its coordinator, +/// so nothing accumulates over a DEBUG session and a new coordinator reusing +/// a freed address can never read a predecessor's route. @MainActor private enum WorkspaceListApplyRouteProbe { - private static var lastRoutesByCoordinator: - [ObjectIdentifier: WorkspaceListTableCoordinator.PayloadApplyRoute] = [:] + private final class RouteBox { + let route: WorkspaceListTableCoordinator.PayloadApplyRoute + init(_ route: WorkspaceListTableCoordinator.PayloadApplyRoute) { + self.route = route + } + } + + private static let lastRoutesByCoordinator = + NSMapTable.weakToStrongObjects() static func record( _ route: WorkspaceListTableCoordinator.PayloadApplyRoute, for coordinator: WorkspaceListTableCoordinator ) { - lastRoutesByCoordinator[ObjectIdentifier(coordinator)] = route + lastRoutesByCoordinator.setObject(RouteBox(route), forKey: coordinator) } static func lastRoute( for coordinator: WorkspaceListTableCoordinator ) -> WorkspaceListTableCoordinator.PayloadApplyRoute? { - lastRoutesByCoordinator[ObjectIdentifier(coordinator)] + lastRoutesByCoordinator.object(forKey: coordinator)?.route } } #endif