diff --git a/supacode/Features/DiffView/DiffWindowContentView.swift b/supacode/Features/DiffView/DiffWindowContentView.swift index 23d17a8b..54c0b331 100644 --- a/supacode/Features/DiffView/DiffWindowContentView.swift +++ b/supacode/Features/DiffView/DiffWindowContentView.swift @@ -119,13 +119,14 @@ struct DiffWindowContentView: View { // render failure works by recreating the view with a new identity. .id(state.renderGeneration) .overlay { - if state.isRenderingDiff { + switch state.renderState { + case .rendering: ProgressView() .controlSize(.small) .padding(12) .background(.regularMaterial, in: Circle()) .transition(.opacity) - } else if let renderError = state.renderError { + case .failed(let renderError): VStack(spacing: 6) { Image(systemName: "exclamationmark.triangle") .foregroundStyle(.orange) @@ -138,10 +139,11 @@ struct DiffWindowContentView: View { .frame(maxWidth: 240) .background(.regularMaterial, in: RoundedRectangle(cornerRadius: 8)) .transition(.opacity) + case .idle: + EmptyView() } } - .animation(.easeInOut(duration: 0.15), value: state.isRenderingDiff) - .animation(.easeInOut(duration: 0.15), value: state.renderError) + .animation(.easeInOut(duration: 0.15), value: state.renderState) } else if state.isLoadingFiles { ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) diff --git a/supacode/Features/DiffView/DiffWindowState.swift b/supacode/Features/DiffView/DiffWindowState.swift index d52a6f0f..d11e49af 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -4,21 +4,28 @@ import YiTong @Observable @MainActor final class DiffWindowState { + /// Render phase of the current `diffDocument` in the WebView-backed + /// `DiffView`. Rendering can take noticeably longer than the cache lookup for + /// large files, since diffing/painting happens on the JS side; the phase is + /// driven by the view's `didRender`/`didFail` events. + enum RenderState: Equatable { + case idle + case rendering + case failed(DiffError) + + var isFailed: Bool { + if case .failed = self { return true } + return false + } + } + var worktreeURL: URL? var branchName: String = "" var changedFiles: [DiffChangedFile] = [] var selectedFile: DiffChangedFile? var diffDocument: DiffDocument? var isLoadingFiles = false - /// True while the WebView-backed `DiffView` is still rendering the current - /// `diffDocument` — this can take noticeably longer than the cache lookup for - /// large files, since diffing/painting happens on the JS side. Cleared by - /// `markDiffRendered()` once the view reports its `didRender` event. - var isRenderingDiff = false - /// Set by `markDiffFailed(_:)` when `DiffView` reports a `.didFail` event, so - /// the render-in-progress indicator doesn't stay stuck forever. Cleared as soon - /// as a new document starts rendering. - var renderError: DiffError? + var renderState: RenderState = .idle /// Identity for the hosted `DiffView` (used as `.id()` by the view). YiTong /// skips re-rendering a value-equal document, so after a render failure the /// only way to retry the same content is to recreate the view; bumping this @@ -27,24 +34,21 @@ final class DiffWindowState { private var documentCache: [String: DiffDocument] = [:] private var loadTask: Task? - private var selectDebounceTask: Task? + private let selectDebouncer: Debouncer private let fetchChangedFiles: @Sendable (URL) async -> [DiffChangedFile] private let loadDiffDocument: @Sendable (DiffChangedFile, URL) async -> DiffDocument - private let selectDebounceInterval: Duration - private let sleep: @Sendable (Duration) async throws -> Void - init>( + init( fetchChangedFiles: @escaping @Sendable (URL) async -> [DiffChangedFile] = DiffWindowState.liveFetchChangedFiles, loadDiffDocument: @escaping @Sendable (DiffChangedFile, URL) async -> DiffDocument = DiffWindowState .liveLoadDocument, selectDebounceInterval: Duration = .milliseconds(150), - clock: C = ContinuousClock() + clock: any Clock = ContinuousClock() ) { self.fetchChangedFiles = fetchChangedFiles self.loadDiffDocument = loadDiffDocument - self.selectDebounceInterval = selectDebounceInterval - self.sleep = { duration in try await clock.sleep(for: duration) } + self.selectDebouncer = Debouncer(interval: selectDebounceInterval, clock: clock) } func load(worktreeURL: URL, branchName: String) { @@ -54,8 +58,7 @@ final class DiffWindowState { selectedFile = nil diffDocument = nil documentCache = [:] - selectDebounceTask?.cancel() - selectDebounceTask = nil + selectDebouncer.cancel() loadTask?.cancel() loadTask = Task { await loadAllFiles(worktreeURL: worktreeURL) } } @@ -70,48 +73,37 @@ final class DiffWindowState { func selectFile(_ file: DiffChangedFile) { // Re-selecting the current file is a no-op unless its render failed, in // which case it is the natural retry gesture. - guard selectedFile != file || renderError != nil else { return } + guard selectedFile != file || renderState.isFailed else { return } selectedFile = file // Leading-edge debounce: a deliberate selection applies immediately, but it // opens a window during which rapid follow-up selections are deferred — so // flicking through files (A -> B -> C) only renders the endpoints, never the // files the user just passed through. - let applyImmediately = selectDebounceTask == nil - if applyImmediately { + if selectDebouncer.isIdle { updateDiffDocument(documentCache[file.id]) - } - selectDebounceTask?.cancel() - let sleep = self.sleep - let interval = selectDebounceInterval - selectDebounceTask = Task { [weak self, sleep] in - do { - try await sleep(interval) - } catch { - return + // Opens the coalescing window; nothing to re-apply when it closes. + selectDebouncer.schedule {} + } else { + selectDebouncer.schedule { [weak self] in + // The selection may have changed via a path other than `selectFile` while + // the window was open (e.g. `loadAllFiles` reconciliation after a refresh) + // — only apply the deferred document if `file` is still current. + guard let self, self.selectedFile?.id == file.id else { return } + self.updateDiffDocument(self.documentCache[file.id]) } - guard let self, !Task.isCancelled else { return } - self.selectDebounceTask = nil - // A leading-edge task only marks the end of the debounce window. - guard !applyImmediately else { return } - // The selection may have changed via a path other than `selectFile` while this - // task was waiting (e.g. `loadAllFiles` reconciliation after a refresh) — only - // apply this debounced document if `file` is still the current selection. - guard self.selectedFile?.id == file.id else { return } - self.updateDiffDocument(self.documentCache[file.id]) } } /// Called by the view once `DiffView` reports its `didRender` event. func markDiffRendered() { - isRenderingDiff = false + renderState = .idle } /// Called by the view once `DiffView` reports its `didFail` event, so the /// loading indicator doesn't stay stuck forever when a render fails. func markDiffFailed(_ error: DiffError) { - isRenderingDiff = false - renderError = error + renderState = .failed(error) } private func updateDiffDocument(_ newDocument: DiffDocument?) { @@ -120,16 +112,12 @@ final class DiffWindowState { // failure it means the user asked for a retry (refresh, or re-selecting // the failed file). YiTong won't re-render an equal document, so force // the view to be recreated instead. - guard renderError != nil, newDocument != nil else { return } - renderError = nil - isRenderingDiff = true + guard renderState.isFailed, newDocument != nil else { return } + renderState = .rendering renderGeneration += 1 return } - isRenderingDiff = newDocument != nil - if isRenderingDiff { - renderError = nil - } + renderState = newDocument != nil ? .rendering : .idle diffDocument = newDocument } diff --git a/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift b/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift index ca570830..571a62d4 100644 --- a/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift +++ b/supacode/Features/Repositories/BusinessLogic/PullRequestRefreshCoordinator.swift @@ -81,17 +81,16 @@ final class PullRequestRefreshCoordinator { private let githubCLI: GithubCLIClient private let clock: any Clock - private let debounceWindow: Duration private let softTimeout: Duration private let resultHandler: @MainActor (Outcome) -> Void - private struct BatchKey: Hashable, Sendable { + private nonisolated struct BatchKey: Hashable, Sendable { let host: String let accountOverride: GithubAccountOverride? } private var pendingByHost: [BatchKey: [Repository.ID: Request]] = [:] - private var flushTaskByHost: [BatchKey: Task] = [:] + private let flushDebouncer: KeyedDebouncer private var inflightHosts: Set = [] private var queuedByHost: [BatchKey: [Repository.ID: Request]] = [:] @@ -104,9 +103,9 @@ final class PullRequestRefreshCoordinator { ) { self.githubCLI = githubCLI self.clock = clock - self.debounceWindow = debounceWindow self.softTimeout = softTimeout self.resultHandler = resultHandler + flushDebouncer = KeyedDebouncer(interval: debounceWindow, clock: clock) } func enqueue(_ request: Request) { @@ -143,19 +142,14 @@ final class PullRequestRefreshCoordinator { } func cancelHost(_ host: String) { - for key in flushTaskByHost.keys where key.host == host { - flushTaskByHost.removeValue(forKey: key)?.cancel() - } + flushDebouncer.cancelAll { $0.host == host } pendingByHost = pendingByHost.filter { $0.key.host != host } queuedByHost = queuedByHost.filter { $0.key.host != host } inflightHosts = inflightHosts.filter { $0.host != host } } func reset() { - for (_, task) in flushTaskByHost { - task.cancel() - } - flushTaskByHost.removeAll() + flushDebouncer.cancelAll() pendingByHost.removeAll() queuedByHost.removeAll() inflightHosts.removeAll() @@ -199,20 +193,13 @@ final class PullRequestRefreshCoordinator { } private func rescheduleDebounce(forKey key: BatchKey) { - flushTaskByHost.removeValue(forKey: key)?.cancel() - let task = Task { [weak self, debounceWindow, clock] in - do { - try await clock.sleep(for: debounceWindow) - } catch { - return - } + flushDebouncer.schedule(key) { [weak self] in await self?.flush(key: key) } - flushTaskByHost[key] = task } private func flush(key: BatchKey) async { - flushTaskByHost.removeValue(forKey: key) + flushDebouncer.cancel(key) guard let bucket = pendingByHost.removeValue(forKey: key), !bucket.isEmpty else { return } diff --git a/supacode/Features/Repositories/BusinessLogic/WorktreeInfoWatcherManager.swift b/supacode/Features/Repositories/BusinessLogic/WorktreeInfoWatcherManager.swift index ee6188a8..f283f2d5 100644 --- a/supacode/Features/Repositories/BusinessLogic/WorktreeInfoWatcherManager.swift +++ b/supacode/Features/Repositories/BusinessLogic/WorktreeInfoWatcherManager.swift @@ -95,14 +95,13 @@ final class WorktreeInfoWatcherManager { private var worktreeFileEventMonitors: [Worktree.ID: WorktreeFileEventMonitoring] = [:] private var worktreeRegistryMonitors: [URL: WorktreeRegistryMonitoring] = [:] private var remoteConfigMonitors: [URL: RemoteConfigMonitoring] = [:] - private var branchDebounceTasks: [Worktree.ID: Task] = [:] - private var filesDebounceTasks: [Worktree.ID: Task] = [:] - private var repositoryWorktreesDebounceTasks: [URL: Task] = [:] - private var repositoryRemoteConfigDebounceTasks: [URL: Task] = [:] - private var restartTasks: [Worktree.ID: Task] = [:] + private let branchChangedDebouncer: KeyedDebouncer + private let repositoryWorktreesDebouncer: KeyedDebouncer + private let remoteConfigDebouncer: KeyedDebouncer + private let restartDebouncer: KeyedDebouncer + private let lineChangesRefreshDebouncer: KeyedDebouncer private var pullRequestTasks: [URL: RefreshTask] = [:] private var lineChangeSafetyTasks: [Worktree.ID: RefreshTask] = [:] - private var lineChangeRefreshTasks: [Worktree.ID: Task] = [:] private var deferredLineChangeIDs: Set = [] private var openedWorktreeIDs: Set = [] private var hasCompletedInitialWorktreeLoad = false @@ -149,6 +148,13 @@ final class WorktreeInfoWatcherManager { self.sleep = { duration in try await clock.sleep(for: duration) } + branchChangedDebouncer = KeyedDebouncer(interval: .milliseconds(200), clock: clock) + repositoryWorktreesDebouncer = KeyedDebouncer(interval: repositoryWorktreesEventDebounceInterval, clock: clock) + remoteConfigDebouncer = KeyedDebouncer(interval: remoteConfigEventDebounceInterval, clock: clock) + restartDebouncer = KeyedDebouncer(interval: .seconds(5), clock: clock) + // Callers always pass an explicit per-worktree delay; the instance interval + // is only a nominal default. + lineChangesRefreshDebouncer = KeyedDebouncer(interval: defaultLineChangesTiming.eventDebounce, clock: clock) } func handleCommand(_ command: WorktreeInfoWatcherClient.Command) { @@ -322,15 +328,9 @@ final class WorktreeInfoWatcherManager { } private func scheduleBranchChanged(worktreeID: Worktree.ID) { - branchDebounceTasks[worktreeID]?.cancel() - let sleep = self.sleep - let task = Task { [weak self, sleep] in - try? await sleep(.milliseconds(200)) - await MainActor.run { - self?.emit(.branchChanged(worktreeID: worktreeID)) - } + branchChangedDebouncer.schedule(worktreeID) { [weak self] in + self?.emit(.branchChanged(worktreeID: worktreeID)) } - branchDebounceTasks[worktreeID] = task } private func scheduleFilesChanged(worktreeID: Worktree.ID) { @@ -342,15 +342,9 @@ final class WorktreeInfoWatcherManager { } private func scheduleRestart(worktreeID: Worktree.ID) { - restartTasks[worktreeID]?.cancel() - let sleep = self.sleep - let task = Task { [weak self, sleep] in - try? await sleep(.seconds(5)) - await MainActor.run { - self?.restartWatcher(worktreeID: worktreeID) - } + restartDebouncer.schedule(worktreeID) { [weak self] in + self?.restartWatcher(worktreeID: worktreeID) } - restartTasks[worktreeID] = task } private func restartWatcher(worktreeID: Worktree.ID) { @@ -373,35 +367,27 @@ final class WorktreeInfoWatcherManager { private func stopWatcher(for worktreeID: Worktree.ID) { stopHeadWatcher(for: worktreeID) stopWorktreeFileEventMonitor(for: worktreeID) - branchDebounceTasks.removeValue(forKey: worktreeID)?.cancel() - filesDebounceTasks.removeValue(forKey: worktreeID)?.cancel() - restartTasks.removeValue(forKey: worktreeID)?.cancel() + branchChangedDebouncer.cancel(worktreeID) + restartDebouncer.cancel(worktreeID) lineChangeSafetyTasks.removeValue(forKey: worktreeID)?.task.cancel() - lineChangeRefreshTasks.removeValue(forKey: worktreeID)?.cancel() + lineChangesRefreshDebouncer.cancel(worktreeID) } private func stopAll() { for watcher in headWatchers.values { watcher.monitor.cancel() } - for task in branchDebounceTasks.values { - task.cancel() - } - for task in filesDebounceTasks.values { - task.cancel() - } - for task in restartTasks.values { - task.cancel() - } + branchChangedDebouncer.cancelAll() + restartDebouncer.cancelAll() + lineChangesRefreshDebouncer.cancelAll() + repositoryWorktreesDebouncer.cancelAll() + remoteConfigDebouncer.cancelAll() for task in pullRequestTasks.values { task.task.cancel() } for task in lineChangeSafetyTasks.values { task.task.cancel() } - for task in lineChangeRefreshTasks.values { - task.cancel() - } for monitor in worktreeFileEventMonitors.values { monitor.cancel() } @@ -411,24 +397,12 @@ final class WorktreeInfoWatcherManager { for monitor in remoteConfigMonitors.values { monitor.cancel() } - for task in repositoryWorktreesDebounceTasks.values { - task.cancel() - } - for task in repositoryRemoteConfigDebounceTasks.values { - task.cancel() - } headWatchers.removeAll() worktreeFileEventMonitors.removeAll() worktreeRegistryMonitors.removeAll() remoteConfigMonitors.removeAll() - branchDebounceTasks.removeAll() - filesDebounceTasks.removeAll() - repositoryWorktreesDebounceTasks.removeAll() - repositoryRemoteConfigDebounceTasks.removeAll() - restartTasks.removeAll() pullRequestTasks.removeAll() lineChangeSafetyTasks.removeAll() - lineChangeRefreshTasks.removeAll() deferredLineChangeIDs.removeAll() openedWorktreeIDs.removeAll() hasCompletedInitialWorktreeLoad = false @@ -532,20 +506,9 @@ final class WorktreeInfoWatcherManager { guard worktrees[worktreeID] != nil else { return } - lineChangeRefreshTasks[worktreeID]?.cancel() - let sleep = self.sleep - let task = Task { [weak self, sleep] in - do { - try await sleep(delay) - } catch { - return - } - await MainActor.run { - self?.lineChangeRefreshTasks.removeValue(forKey: worktreeID) - self?.emitLineChangesChanged(worktreeID: worktreeID) - } + lineChangesRefreshDebouncer.schedule(worktreeID, after: delay) { [weak self] in + self?.emitLineChangesChanged(worktreeID: worktreeID) } - lineChangeRefreshTasks[worktreeID] = task } private func scheduleLineChangesDebouncedRefresh(worktreeID: Worktree.ID) { @@ -645,7 +608,7 @@ final class WorktreeInfoWatcherManager { let obsoleteRoots = worktreeRegistryMonitors.keys.filter { !normalizedRoots.contains($0) } for repositoryRootURL in obsoleteRoots { worktreeRegistryMonitors.removeValue(forKey: repositoryRootURL)?.cancel() - repositoryWorktreesDebounceTasks.removeValue(forKey: repositoryRootURL)?.cancel() + repositoryWorktreesDebouncer.cancel(repositoryRootURL) } for repositoryRootURL in normalizedRoots where worktreeRegistryMonitors[repositoryRootURL] == nil { worktreeRegistryMonitors[repositoryRootURL] = worktreeRegistryMonitorFactory(repositoryRootURL) { [weak self] in @@ -659,7 +622,7 @@ final class WorktreeInfoWatcherManager { let obsoleteRoots = remoteConfigMonitors.keys.filter { !normalizedRoots.contains($0) } for repositoryRootURL in obsoleteRoots { remoteConfigMonitors.removeValue(forKey: repositoryRootURL)?.cancel() - repositoryRemoteConfigDebounceTasks.removeValue(forKey: repositoryRootURL)?.cancel() + remoteConfigDebouncer.cancel(repositoryRootURL) } for repositoryRootURL in normalizedRoots where remoteConfigMonitors[repositoryRootURL] == nil { remoteConfigMonitors[repositoryRootURL] = remoteConfigMonitorFactory(repositoryRootURL) { [weak self] in @@ -670,40 +633,16 @@ final class WorktreeInfoWatcherManager { private func scheduleRepositoryWorktreesChanged(repositoryRootURL: URL) { let normalizedRootURL = repositoryRootURL.standardizedFileURL - repositoryWorktreesDebounceTasks[normalizedRootURL]?.cancel() - let debounceInterval = repositoryWorktreesEventDebounceInterval - let sleep = self.sleep - let task = Task { [weak self, sleep] in - do { - try await sleep(debounceInterval) - } catch { - return - } - await MainActor.run { - self?.repositoryWorktreesDebounceTasks.removeValue(forKey: normalizedRootURL) - self?.emit(.repositoryWorktreesChanged(repositoryRootURL: normalizedRootURL)) - } + repositoryWorktreesDebouncer.schedule(normalizedRootURL) { [weak self] in + self?.emit(.repositoryWorktreesChanged(repositoryRootURL: normalizedRootURL)) } - repositoryWorktreesDebounceTasks[normalizedRootURL] = task } private func scheduleRepositoryRemoteConfigurationChanged(repositoryRootURL: URL) { let normalizedRootURL = repositoryRootURL.standardizedFileURL - repositoryRemoteConfigDebounceTasks[normalizedRootURL]?.cancel() - let debounceInterval = remoteConfigEventDebounceInterval - let sleep = self.sleep - let task = Task { [weak self, sleep] in - do { - try await sleep(debounceInterval) - } catch { - return - } - await MainActor.run { - self?.repositoryRemoteConfigDebounceTasks.removeValue(forKey: normalizedRootURL) - self?.emit(.repositoryRemoteConfigurationChanged(repositoryRootURL: normalizedRootURL)) - } + remoteConfigDebouncer.schedule(normalizedRootURL) { [weak self] in + self?.emit(.repositoryRemoteConfigurationChanged(repositoryRootURL: normalizedRootURL)) } - repositoryRemoteConfigDebounceTasks[normalizedRootURL] = task } private func updateRepeatingTask( diff --git a/supacode/Support/Debouncer.swift b/supacode/Support/Debouncer.swift new file mode 100644 index 00000000..f3293732 --- /dev/null +++ b/supacode/Support/Debouncer.swift @@ -0,0 +1,87 @@ +import Foundation + +/// Coalesces bursts of calls into a single delayed action: each `schedule` +/// replaces the pending action and restarts the interval, and a cancelled +/// action never fires. +/// +/// Centralizes the two easy-to-miss parts of the hand-rolled pattern: catching +/// the sleep's `CancellationError` *and* re-checking `Task.isCancelled` once +/// the sleep resumes, so a cancelled task can't slip through and run anyway. +@MainActor +final class Debouncer { + private let interval: Duration + private let clock: any Clock + private var task: Task? + + init(interval: Duration, clock: any Clock = ContinuousClock()) { + self.interval = interval + self.clock = clock + } + + /// True when no action is pending, i.e. the next `schedule` starts a fresh + /// debounce window. Callers implementing leading-edge behavior check this to + /// decide whether to act immediately instead. + var isIdle: Bool { task == nil } + + /// Schedules `action` to run after the interval, replacing any pending action. + func schedule(_ action: @escaping @MainActor () async -> Void) { + task?.cancel() + task = Task { [interval, clock] in + do { + try await clock.sleep(for: interval) + } catch { + return + } + guard !Task.isCancelled else { return } + self.task = nil + await action() + } + } + + func cancel() { + task?.cancel() + task = nil + } +} + +/// A `Debouncer` variant that maintains an independent debounce window per key, +/// for stores that coalesce events per worktree, repository, or host. +@MainActor +final class KeyedDebouncer { + private let interval: Duration + private let clock: any Clock + private var tasks: [Key: Task] = [:] + + init(interval: Duration, clock: any Clock = ContinuousClock()) { + self.interval = interval + self.clock = clock + } + + /// Schedules `action` for `key` after `interval` (defaulting to the instance + /// interval), replacing any action already pending for the same key. + func schedule(_ key: Key, after interval: Duration? = nil, _ action: @escaping @MainActor () async -> Void) { + tasks[key]?.cancel() + let interval = interval ?? self.interval + tasks[key] = Task { [clock] in + do { + try await clock.sleep(for: interval) + } catch { + return + } + guard !Task.isCancelled else { return } + self.tasks.removeValue(forKey: key) + await action() + } + } + + func cancel(_ key: Key) { + tasks.removeValue(forKey: key)?.cancel() + } + + /// Cancels every pending action, or only those whose key matches `shouldCancel`. + func cancelAll(where shouldCancel: (Key) -> Bool = { _ in true }) { + for key in Array(tasks.keys) where shouldCancel(key) { + tasks.removeValue(forKey: key)?.cancel() + } + } +} diff --git a/supacodeTests/DebouncerTests.swift b/supacodeTests/DebouncerTests.swift new file mode 100644 index 00000000..5c9049bf --- /dev/null +++ b/supacodeTests/DebouncerTests.swift @@ -0,0 +1,147 @@ +import Clocks +import Foundation +import Testing + +@testable import supacode + +@MainActor +struct DebouncerTests { + @Test func actionFiresAfterInterval() async { + let clock = TestClock() + let debouncer = Debouncer(interval: .milliseconds(100), clock: clock) + var fired = false + + debouncer.schedule { fired = true } + await advance(clock, by: .milliseconds(99)) + #expect(!fired) + + await advance(clock, by: .milliseconds(1)) + #expect(fired) + } + + @Test func reschedulingReplacesPendingActionAndRestartsInterval() async { + let clock = TestClock() + let debouncer = Debouncer(interval: .milliseconds(100), clock: clock) + var firstFired = false + var secondFired = false + + debouncer.schedule { firstFired = true } + await advance(clock, by: .milliseconds(60)) + debouncer.schedule { secondFired = true } + await advance(clock, by: .milliseconds(60)) + #expect(!firstFired) + #expect(!secondFired) + + await advance(clock, by: .milliseconds(40)) + #expect(!firstFired) + #expect(secondFired) + } + + @Test func cancelledActionNeverFires() async { + // The hand-rolled pattern this type replaces was easy to get wrong: a + // `try? await sleep` swallows the cancellation error and the body runs + // anyway, turning "cancel" into "fire immediately". + let clock = TestClock() + let debouncer = Debouncer(interval: .milliseconds(100), clock: clock) + var fired = false + + debouncer.schedule { fired = true } + await advance(clock, by: .milliseconds(50)) + debouncer.cancel() + await advance(clock, by: .milliseconds(200)) + + #expect(!fired) + } + + @Test func isIdleTracksThePendingWindow() async { + let clock = TestClock() + let debouncer = Debouncer(interval: .milliseconds(100), clock: clock) + #expect(debouncer.isIdle) + + debouncer.schedule {} + #expect(!debouncer.isIdle) + + await advance(clock, by: .milliseconds(100)) + #expect(debouncer.isIdle) + + debouncer.schedule {} + debouncer.cancel() + #expect(debouncer.isIdle) + } +} + +@MainActor +struct KeyedDebouncerTests { + @Test func keysDebounceIndependently() async { + let clock = TestClock() + let debouncer = KeyedDebouncer(interval: .milliseconds(100), clock: clock) + var fired: [String] = [] + + debouncer.schedule("a") { fired.append("a") } + await advance(clock, by: .milliseconds(50)) + debouncer.schedule("b") { fired.append("b") } + // Rescheduling "a" must not affect "b"'s window. + debouncer.schedule("a") { fired.append("a2") } + + await advance(clock, by: .milliseconds(100)) + #expect(fired.sorted() == ["a2", "b"]) + } + + @Test func perCallIntervalOverridesDefault() async { + let clock = TestClock() + let debouncer = KeyedDebouncer(interval: .seconds(10), clock: clock) + var fired = false + + debouncer.schedule("a", after: .milliseconds(100)) { fired = true } + await advance(clock, by: .milliseconds(100)) + + #expect(fired) + } + + @Test func cancelOnlyAffectsTheGivenKey() async { + let clock = TestClock() + let debouncer = KeyedDebouncer(interval: .milliseconds(100), clock: clock) + var fired: [String] = [] + + debouncer.schedule("a") { fired.append("a") } + debouncer.schedule("b") { fired.append("b") } + debouncer.cancel("a") + await advance(clock, by: .milliseconds(100)) + + #expect(fired == ["b"]) + } + + @Test func cancelAllSupportsSelectivePredicate() async { + let clock = TestClock() + let debouncer = KeyedDebouncer(interval: .milliseconds(100), clock: clock) + var fired: [String] = [] + + debouncer.schedule("keep") { fired.append("keep") } + debouncer.schedule("drop-1") { fired.append("drop-1") } + debouncer.schedule("drop-2") { fired.append("drop-2") } + debouncer.cancelAll { $0.hasPrefix("drop") } + await advance(clock, by: .milliseconds(100)) + + #expect(fired == ["keep"]) + } + + @Test func cancelAllCancelsEverything() async { + let clock = TestClock() + let debouncer = KeyedDebouncer(interval: .milliseconds(100), clock: clock) + var fired: [String] = [] + + debouncer.schedule("a") { fired.append("a") } + debouncer.schedule("b") { fired.append("b") } + debouncer.cancelAll() + await advance(clock, by: .milliseconds(200)) + + #expect(fired.isEmpty) + } +} + +@MainActor +private func advance(_ clock: TestClock, by duration: Duration) async { + await Task.yield() + await clock.advance(by: duration) + await Task.yield() +} diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift index 24a6441c..ad1a91e2 100644 --- a/supacodeTests/DiffWindowStateTests.swift +++ b/supacodeTests/DiffWindowStateTests.swift @@ -134,7 +134,7 @@ struct DiffWindowStateTests { state.selectFile(fileB) await advanceSelectDebounce(clock) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) #expect(state.diffDocument == docB) } @@ -154,7 +154,7 @@ struct DiffWindowStateTests { state.selectFile(fileB) await advanceSelectDebounce(clock) - #expect(!state.isRenderingDiff) + #expect(state.renderState == .idle) } @Test func loadAllFilesMarksRenderingWhenAutoSelectedDocumentArrives() async { @@ -167,7 +167,7 @@ struct DiffWindowStateTests { await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) } @Test func selectFileAppliesCachedDocumentImmediately() async { @@ -189,7 +189,7 @@ struct DiffWindowStateTests { state.selectFile(fileB) #expect(state.diffDocument == docB) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) } @Test func selectFileDefersFollowUpSelectionWithinDebounceWindow() async { @@ -301,7 +301,7 @@ struct DiffWindowStateTests { #expect(state.diffDocument != docC) } - @Test func markDiffFailedClearsRenderingAndStoresError() async { + @Test func markDiffFailedStoresFailedState() async { let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") let docA = DiffDocument(files: [], title: "a") let state = DiffWindowState( @@ -309,13 +309,12 @@ struct DiffWindowStateTests { loadDiffDocument: { _, _ in docA } ) await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) let error = DiffError(code: "render_failed", message: "boom") state.markDiffFailed(error) - #expect(!state.isRenderingDiff) - #expect(state.renderError == error) + #expect(state.renderState == .failed(error)) } @Test func selectingANewFileClearsAPriorRenderError() async { @@ -332,12 +331,12 @@ struct DiffWindowStateTests { ) await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) state.markDiffFailed(DiffError(code: "render_failed", message: "boom")) - #expect(state.renderError != nil) + #expect(state.renderState.isFailed) state.selectFile(fileB) await advanceSelectDebounce(clock) - #expect(state.renderError == nil) + #expect(state.renderState == .rendering) } @Test func reselectingFailedFileRetriesRender() async { @@ -357,8 +356,7 @@ struct DiffWindowStateTests { state.selectFile(fileA) - #expect(state.renderError == nil) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) #expect(state.renderGeneration == generationBefore + 1) #expect(state.diffDocument == docA) } @@ -379,8 +377,7 @@ struct DiffWindowStateTests { // unchanged so the reloaded document is value-equal to the current one. await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) - #expect(state.renderError == nil) - #expect(state.isRenderingDiff) + #expect(state.renderState == .rendering) #expect(state.renderGeneration == generationBefore + 1) #expect(state.diffDocument == docA) } @@ -400,7 +397,7 @@ struct DiffWindowStateTests { state.selectFile(fileA) - #expect(!state.isRenderingDiff) + #expect(state.renderState == .idle) #expect(state.renderGeneration == generationBefore) } }