Skip to content

Commit 452d8d0

Browse files
Fix UI freezes during rapid unsubscribes.
Coalesce expensive inbox rebuild work with fast dismissal checks, deferred action persistence, and shared bulk-action code so quick unsub bursts stay responsive on the main thread. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0f34686 commit 452d8d0

6 files changed

Lines changed: 285 additions & 162 deletions

File tree

Octodot.xcodeproj/project.pbxproj

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
BB000000000000000000002B /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000000000000000000002C /* SettingsView.swift */; };
3232
BB000000000000000000002D /* HotkeyRecorderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000000000000000000002E /* HotkeyRecorderView.swift */; };
3333
BB0000000000000000000030 /* InboxStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0000000000000000000030 /* InboxStore.swift */; };
34+
BB0000000000000000000037 /* PersistenceCoding.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0000000000000000000037 /* PersistenceCoding.swift */; };
3435
BB0000000000000000000032 /* SemanticVersion.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0000000000000000000032 /* SemanticVersion.swift */; };
3536
BB0000000000000000000033 /* UpdateChecker.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0000000000000000000033 /* UpdateChecker.swift */; };
3637
BB0000000000000000000036 /* UpdateInstaller.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA0000000000000000000036 /* UpdateInstaller.swift */; };
@@ -80,6 +81,7 @@
8081
AA000000000000000000002D /* SettingsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsTests.swift; sourceTree = "<group>"; };
8182
AA000000000000000000002E /* HotkeyRecorderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HotkeyRecorderView.swift; sourceTree = "<group>"; };
8283
AA0000000000000000000030 /* InboxStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InboxStore.swift; sourceTree = "<group>"; };
84+
AA0000000000000000000037 /* PersistenceCoding.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceCoding.swift; sourceTree = "<group>"; };
8385
AA0000000000000000000032 /* SemanticVersion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SemanticVersion.swift; sourceTree = "<group>"; };
8486
AA0000000000000000000033 /* UpdateChecker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateChecker.swift; sourceTree = "<group>"; };
8587
AA0000000000000000000036 /* UpdateInstaller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateInstaller.swift; sourceTree = "<group>"; };
@@ -135,6 +137,7 @@
135137
AA000000000000000000002B /* AppPreferences.swift */,
136138
AA0000000000000000000003 /* AppState.swift */,
137139
AA0000000000000000000030 /* InboxStore.swift */,
140+
AA0000000000000000000037 /* PersistenceCoding.swift */,
138141
AA0000000000000000000032 /* SemanticVersion.swift */,
139142
AA0000000000000000000029 /* ThreadActionStore.swift */,
140143
AA0000000000000000000033 /* UpdateChecker.swift */,
@@ -346,6 +349,7 @@
346349
BB000000000000000000002A /* AppPreferences.swift in Sources */,
347350
BB0000000000000000000003 /* AppState.swift in Sources */,
348351
BB0000000000000000000030 /* InboxStore.swift in Sources */,
352+
BB0000000000000000000037 /* PersistenceCoding.swift in Sources */,
349353
BB0000000000000000000028 /* ThreadActionStore.swift in Sources */,
350354
BB0000000000000000000004 /* StatusItemController.swift in Sources */,
351355
BB0000000000000000000005 /* NotificationPanel.swift in Sources */,
@@ -451,7 +455,7 @@
451455
MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE;
452456
MTL_FAST_MATH = YES;
453457
OCTODOT_BUILD_NUMBER = 1;
454-
OCTODOT_MARKETING_VERSION = 0.5.8;
458+
OCTODOT_MARKETING_VERSION = 0.5.9;
455459
ONLY_ACTIVE_ARCH = YES;
456460
SDKROOT = macosx;
457461
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "$(inherited) DEBUG";
@@ -508,7 +512,7 @@
508512
MACOSX_DEPLOYMENT_TARGET = 14.0;
509513
MTL_FAST_MATH = YES;
510514
OCTODOT_BUILD_NUMBER = 1;
511-
OCTODOT_MARKETING_VERSION = 0.5.8;
515+
OCTODOT_MARKETING_VERSION = 0.5.9;
512516
SDKROOT = macosx;
513517
SWIFT_COMPILATION_MODE = wholemodule;
514518
SWIFT_OPTIMIZATION_LEVEL = "-O";

Octodot/App/AppState.swift

Lines changed: 131 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import Observation
33

44
private let defaultActionDispatchDelayNanoseconds: UInt64 = 2_500_000_000
55
private let defaultBackgroundRefreshFallbackNanoseconds: UInt64 = 60_000_000_000
6+
private let deferredPersistenceNanoseconds: UInt64 = 100_000_000
67

78
private let defaultSleepHandler: AppState.SleepHandler = { nanoseconds in
89
guard nanoseconds > 0 else { return }
@@ -109,6 +110,7 @@ final class AppState {
109110
private var threadActions: ThreadActionStore
110111
private var actionTasks: [String: Task<Void, Never>] = [:]
111112
private var batchDispatchTask: Task<Void, Never>?
113+
private var committedActionsPersistTask: Task<Void, Never>?
112114
private var backgroundRefreshTask: Task<Void, Never>?
113115
private var securityAlertsRefreshTask: Task<Void, Never>?
114116
private var subjectStateResolutionTask: Task<Void, Never>?
@@ -513,33 +515,8 @@ final class AppState {
513515
}
514516

515517
func done() {
516-
if let batch = checkedNotificationsBatch() {
517-
let originalVisibleOrder = filteredNotifications
518-
let originalSelectionID = selectedNotificationID
519-
var acceptedItems: [GitHubNotification] = []
520-
clearChecked()
521-
522-
for notification in batch where notification.source == .dependabotAlert {
523-
dismissSecurityAlert(notification, updatesSelection: false)
524-
acceptedItems.append(notification)
525-
}
526-
for group in groupedThreadNotifications(from: batch) {
527-
guard let representative = group.first else { continue }
528-
if startThreadAction(
529-
.done,
530-
target: representative,
531-
activityIdentities: group.map(\.activityIdentity),
532-
updatesSelection: false
533-
) {
534-
acceptedItems.append(contentsOf: group)
535-
}
536-
}
537-
538-
restoreSelectionAfterBulkMutation(
539-
originalSelectionID: originalSelectionID,
540-
originalVisibleOrder: originalVisibleOrder
541-
)
542-
presentActionToast(verb: .done, items: acceptedItems)
518+
if checkedNotificationsBatch() != nil {
519+
performBulkThreadAction(.done)
543520
return
544521
}
545522
guard let target = selectedNotification else { return }
@@ -556,36 +533,8 @@ final class AppState {
556533
}
557534

558535
func unsubscribeFromThread() {
559-
if let batch = checkedNotificationsBatch() {
560-
let originalVisibleOrder = filteredNotifications
561-
let originalSelectionID = selectedNotificationID
562-
let securityAlerts = batch.filter { $0.source == .dependabotAlert }
563-
var unsubscribedItems: [GitHubNotification] = []
564-
clearChecked()
565-
566-
for notification in securityAlerts {
567-
dismissSecurityAlert(notification, updatesSelection: false)
568-
}
569-
for group in groupedThreadNotifications(from: batch) {
570-
guard let representative = group.first else { continue }
571-
if startThreadAction(
572-
.unsubscribe,
573-
target: representative,
574-
activityIdentities: group.map(\.activityIdentity),
575-
updatesSelection: false
576-
) {
577-
inboxStore.muteThread(representative.threadId)
578-
clampSelection()
579-
unsubscribedItems.append(contentsOf: group)
580-
}
581-
}
582-
583-
restoreSelectionAfterBulkMutation(
584-
originalSelectionID: originalSelectionID,
585-
originalVisibleOrder: originalVisibleOrder
586-
)
587-
presentActionToast(verb: .unsub, items: unsubscribedItems)
588-
presentActionToast(verb: .done, items: securityAlerts)
536+
if checkedNotificationsBatch() != nil {
537+
performBulkThreadAction(.unsubscribe)
589538
return
590539
}
591540
guard let notification = selectedNotification else { return }
@@ -599,6 +548,67 @@ final class AppState {
599548
}
600549
}
601550

551+
private enum BulkThreadActionKind {
552+
case done
553+
case unsubscribe
554+
}
555+
556+
private func performBulkThreadAction(_ kind: BulkThreadActionKind) {
557+
guard let batch = checkedNotificationsBatch() else { return }
558+
559+
let originalVisibleOrder = filteredNotifications
560+
let originalSelectionID = selectedNotificationID
561+
var threadItems: [GitHubNotification] = []
562+
var securityAlerts: [GitHubNotification] = []
563+
clearChecked()
564+
565+
switch kind {
566+
case .done:
567+
for notification in batch where notification.source == .dependabotAlert {
568+
dismissSecurityAlert(notification, updatesSelection: false)
569+
threadItems.append(notification)
570+
}
571+
case .unsubscribe:
572+
securityAlerts = batch.filter { $0.source == .dependabotAlert }
573+
for notification in securityAlerts {
574+
dismissSecurityAlert(notification, updatesSelection: false)
575+
}
576+
}
577+
578+
let actionKind: ThreadActionStore.ActionKind = kind == .done ? .done : .unsubscribe
579+
for group in groupedThreadNotifications(from: batch) {
580+
guard let representative = group.first else { continue }
581+
if startThreadAction(
582+
actionKind,
583+
target: representative,
584+
activityIdentities: group.map(\.activityIdentity),
585+
updatesSelection: false
586+
) {
587+
if kind == .unsubscribe {
588+
inboxStore.muteThread(representative.threadId)
589+
}
590+
threadItems.append(contentsOf: group)
591+
}
592+
}
593+
594+
if kind == .unsubscribe {
595+
clampSelection()
596+
}
597+
598+
restoreSelectionAfterBulkMutation(
599+
originalSelectionID: originalSelectionID,
600+
originalVisibleOrder: originalVisibleOrder
601+
)
602+
603+
switch kind {
604+
case .done:
605+
presentActionToast(verb: .done, items: threadItems)
606+
case .unsubscribe:
607+
presentActionToast(verb: .unsub, items: threadItems)
608+
presentActionToast(verb: .done, items: securityAlerts)
609+
}
610+
}
611+
602612
func copyURL() {
603613
guard let notification = selectedNotification else { return }
604614
NSPasteboard.general.clearContents()
@@ -793,6 +803,34 @@ final class AppState {
793803
rebuildDerivedState()
794804
}
795805

806+
private func schedulePersistenceFlush() {
807+
committedActionsPersistTask?.cancel()
808+
committedActionsPersistTask = nil
809+
if threadActions.hasPendingActions {
810+
committedActionsPersistTask = Task { @MainActor [weak self] in
811+
try? await Task.sleep(for: .nanoseconds(deferredPersistenceNanoseconds))
812+
guard !Task.isCancelled else { return }
813+
self?.flushCommittedActionsPersistence()
814+
}
815+
} else {
816+
threadActions.flushCommittedActionsIfNeeded()
817+
}
818+
}
819+
820+
private func flushCommittedActionsPersistence() {
821+
committedActionsPersistTask?.cancel()
822+
committedActionsPersistTask = nil
823+
threadActions.flushCommittedActionsIfNeeded()
824+
}
825+
826+
private func projectThreadActions(from notifications: [GitHubNotification]) -> [GitHubNotification] {
827+
threadActions.projectedNotifications(from: notifications)
828+
}
829+
830+
private func isThreadActionNotificationVisible(_ notification: GitHubNotification) -> Bool {
831+
!threadActions.isNotificationHiddenByDismissal(notification)
832+
}
833+
796834
func flushPendingActions() {
797835
batchDispatchTask?.cancel()
798836
batchDispatchTask = nil
@@ -817,7 +855,10 @@ final class AppState {
817855
) async -> Bool {
818856
flushPendingActions()
819857

820-
guard threadActions.hasPendingActions else { return true }
858+
guard threadActions.hasPendingActions else {
859+
flushCommittedActionsPersistence()
860+
return true
861+
}
821862
guard timeoutNanoseconds > 0 else { return false }
822863

823864
let pollInterval = max(1, min(pollIntervalNanoseconds, timeoutNanoseconds))
@@ -829,22 +870,30 @@ final class AppState {
829870
let sleepNanoseconds = min(pollInterval, remainingNanoseconds)
830871
await terminationSleepHandler(sleepNanoseconds)
831872

832-
guard threadActions.hasPendingActions else { return true }
833-
guard remainingNanoseconds > sleepNanoseconds else { return false }
873+
guard threadActions.hasPendingActions else {
874+
flushCommittedActionsPersistence()
875+
return true
876+
}
877+
guard remainingNanoseconds > sleepNanoseconds else {
878+
flushCommittedActionsPersistence()
879+
return false
880+
}
834881
remainingNanoseconds -= sleepNanoseconds
835882
}
836883

884+
flushCommittedActionsPersistence()
837885
return true
838886
}
839887

840888
private func rebuildDerivedState() {
841-
let projectedUnread = threadActions.projectedNotifications(from: serverNotifications)
842-
let projectedRecentInbox = threadActions.projectedNotifications(from: serverRecentInboxNotifications)
889+
let projectedUnread = projectThreadActions(from: serverNotifications)
890+
let projectedRecentInbox = projectThreadActions(from: serverRecentInboxNotifications)
843891
let projectedSecurityAlerts = inboxStore.projectedSecurityAlerts(from: serverSecurityAlerts)
844892
let modeFiltered = filteredNotificationsForCurrentMode(
845893
unreadNotifications: projectedUnread,
846894
recentInboxNotifications: projectedRecentInbox,
847-
securityAlerts: projectedSecurityAlerts
895+
securityAlerts: projectedSecurityAlerts,
896+
isNotificationVisible: isThreadActionNotificationVisible
848897
)
849898
let repoOrderSource = groupByRepo ? sortedByRecency(serverNotificationsForCurrentMode()) : []
850899
notifications = orderedNotifications(
@@ -894,7 +943,8 @@ final class AppState {
894943
private func filteredNotificationsForCurrentMode(
895944
unreadNotifications: [GitHubNotification],
896945
recentInboxNotifications: [GitHubNotification],
897-
securityAlerts: [GitHubNotification]
946+
securityAlerts: [GitHubNotification],
947+
isNotificationVisible: @escaping (GitHubNotification) -> Bool
898948
) -> [GitHubNotification] {
899949
switch inboxMode {
900950
case .unread:
@@ -903,7 +953,8 @@ final class AppState {
903953
let merged = inboxStore.mergedInboxNotifications(
904954
unreadNotifications: unreadNotifications,
905955
recentInboxNotifications: recentInboxNotifications,
906-
projectedNotifications: { self.threadActions.projectedNotifications(from: $0) }
956+
projectNotifications: projectThreadActions(from:),
957+
isNotificationVisible: isThreadActionNotificationVisible
907958
)
908959
return merged + dedupedSecurityAlerts(securityAlerts, against: merged)
909960
}
@@ -917,7 +968,8 @@ final class AppState {
917968
let merged = inboxStore.mergedInboxNotifications(
918969
unreadNotifications: serverNotifications,
919970
recentInboxNotifications: serverRecentInboxNotifications,
920-
projectedNotifications: { $0 }
971+
projectNotifications: { $0 },
972+
isNotificationVisible: isThreadActionNotificationVisible
921973
)
922974
let securityAlerts = inboxStore.projectedSecurityAlerts(from: serverSecurityAlerts)
923975
return merged + dedupedSecurityAlerts(securityAlerts, against: merged)
@@ -1190,15 +1242,20 @@ final class AppState {
11901242

11911243
private func handlePendingActionSuccess(_ pending: ThreadActionStore.PendingAction) {
11921244
actionTasks[pending.notification.threadId] = nil
1193-
threadActions.handleSuccess(pending, serverNotifications: &serverNotifications)
1245+
threadActions.handleSuccess(
1246+
pending,
1247+
serverNotifications: &serverNotifications,
1248+
deferPersistence: true
1249+
)
1250+
schedulePersistenceFlush()
11941251
switch pending.kind {
11951252
case .markRead:
11961253
var readNotification = pending.notification
11971254
readNotification.isUnread = false
11981255
inboxStore.recordRecentReadNotification(
11991256
readNotification,
12001257
unreadNotifications: serverNotifications.filter(\.isUnread),
1201-
projectedNotifications: { self.threadActions.projectedNotifications(from: $0) }
1258+
isNotificationVisible: isThreadActionNotificationVisible
12021259
)
12031260
case .done, .unsubscribe:
12041261
inboxStore.removeRecentReadNotification(threadId: pending.notification.threadId)
@@ -1509,7 +1566,7 @@ final class AppState {
15091566
// Apply thread action projections so committed done/unsubscribe
15101567
// actions are excluded from the count, matching what the panel shows.
15111568
let projected = inboxStore.filterMutedThreads(
1512-
threadActions.projectedNotifications(from: fetched)
1569+
projectThreadActions(from: fetched)
15131570
)
15141571
unreadNotificationCount = projected.filter(\.isUnread).count
15151572
} catch {
@@ -1588,12 +1645,13 @@ final class AppState {
15881645
serverNotifications = unreadNotifications
15891646
// Filter out threads with committed done/unsubscribe actions so they
15901647
// don't linger in the recent inbox read list after the server confirms removal.
1591-
let projectedRecentInbox = threadActions.projectedNotifications(from: recentInboxNotifications)
1648+
let projectedRecentInbox = projectThreadActions(from: recentInboxNotifications)
15921649
let loadedState = inboxStore.applyLoaded(
15931650
unreadNotifications: unreadNotifications,
15941651
recentInboxNotifications: projectedRecentInbox,
15951652
projectedSecurityAlerts: securityAlerts,
1596-
projectedNotifications: { self.threadActions.projectedNotifications(from: $0) }
1653+
projectNotifications: projectThreadActions(from:),
1654+
isNotificationVisible: isThreadActionNotificationVisible
15971655
)
15981656
serverRecentInboxNotifications = loadedState.recentInboxNotifications
15991657
serverSecurityAlerts = securityAlerts
@@ -1615,7 +1673,8 @@ final class AppState {
16151673
let inboxNotifications = inboxStore.mergedInboxNotifications(
16161674
unreadNotifications: unreadNotifications,
16171675
recentInboxNotifications: recentInboxNotifications,
1618-
projectedNotifications: { self.threadActions.projectedNotifications(from: $0) }
1676+
projectNotifications: projectThreadActions(from:),
1677+
isNotificationVisible: isThreadActionNotificationVisible
16191678
)
16201679
return Array(Set(inboxNotifications.map(\.repository))).sorted()
16211680
}

0 commit comments

Comments
 (0)