diff --git a/supacode/Features/DiffView/DiffWindowState.swift b/supacode/Features/DiffView/DiffWindowState.swift index 27a69496..cd08dbd2 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -14,6 +14,18 @@ final class DiffWindowState { private var documentCache: [String: DiffDocument] = [:] private var loadTask: Task? + private let fetchChangedFiles: @Sendable (URL) async -> [DiffChangedFile] + private let loadDiffDocument: @Sendable (DiffChangedFile, URL) async -> DiffDocument + + init( + fetchChangedFiles: @escaping @Sendable (URL) async -> [DiffChangedFile] = DiffWindowState.liveFetchChangedFiles, + loadDiffDocument: @escaping @Sendable (DiffChangedFile, URL) async -> DiffDocument + = DiffWindowState.liveLoadDocument + ) { + self.fetchChangedFiles = fetchChangedFiles + self.loadDiffDocument = loadDiffDocument + } + func load(worktreeURL: URL, branchName: String) { self.worktreeURL = worktreeURL self.branchName = branchName @@ -27,7 +39,7 @@ final class DiffWindowState { func refresh() { guard let worktreeURL else { return } - documentCache = [:] + // Keep cache intact so file switching remains responsive during refresh loadTask?.cancel() loadTask = Task { await loadAllFiles(worktreeURL: worktreeURL) } } @@ -38,57 +50,85 @@ final class DiffWindowState { diffDocument = documentCache[file.id] } - // MARK: - Private + // MARK: - Reconciliation (pure, testable without hitting Git or Task scheduling) - private func loadAllFiles(worktreeURL: URL) async { - isLoadingFiles = true - async let trackedOutput = GitClient().diffNameStatus(at: worktreeURL) - async let untrackedPaths = GitClient().untrackedFilePaths(at: worktreeURL) - let trackedFiles = DiffChangedFile.parseNameStatus(await trackedOutput) - let untrackedFiles = await untrackedPaths.map { - DiffChangedFile(status: .added, oldPath: nil, newPath: $0) - } - let files = trackedFiles + untrackedFiles - changedFiles = files - - // Load all documents concurrently - let documents = await Self.loadAllDocuments(files: files, worktreeURL: worktreeURL) - guard !Task.isCancelled else { return } - documentCache = documents - isLoadingFiles = false + /// Drops cache entries for files no longer present in the latest changed-file list. + static func evictedCache( + _ cache: [String: DiffDocument], + keeping fileIDs: Set + ) -> [String: DiffDocument] { + cache.filter { fileIDs.contains($0.key) } + } - // Auto-select - if let selectedFile, documents[selectedFile.id] != nil { - diffDocument = documents[selectedFile.id] + /// Keeps the current selection if its document is cached; otherwise falls back to the + /// first file, or to nothing if there are no files. + static func resolvedSelection( + current: DiffChangedFile?, + files: [DiffChangedFile], + cache: [String: DiffDocument] + ) -> (file: DiffChangedFile?, document: DiffDocument?) { + if let current, let document = cache[current.id] { + return (current, document) } else if let first = files.first { - selectedFile = first - diffDocument = documents[first.id] + return (first, cache[first.id]) } else { - selectedFile = nil - diffDocument = nil + return (nil, nil) } } - private nonisolated static func loadAllDocuments( - files: [DiffChangedFile], - worktreeURL: URL - ) async -> [String: DiffDocument] { - await withTaskGroup(of: (String, DiffDocument).self) { group in + // MARK: - Loading + + /// Exposed (not private) so tests can drive it directly with injected fakes, + /// bypassing the `Task` scheduling used by `load()`/`refresh()`. + func loadAllFiles(worktreeURL: URL) async { + isLoadingFiles = true + let files = await fetchChangedFiles(worktreeURL) + + guard !Task.isCancelled else { return } + + changedFiles = files + + let fileIDs = Set(files.map(\.id)) + documentCache = Self.evictedCache(documentCache, keeping: fileIDs) + + // Load documents concurrently, updating the cache as each one completes + // so that file switching is responsive without waiting for all files + await withTaskGroup(of: (String, DiffDocument).self) { [loadDiffDocument] group in for file in files { group.addTask { - let doc = await loadDocument(for: file, worktreeURL: worktreeURL) + let doc = await loadDiffDocument(file, worktreeURL) return (file.id, doc) } } - var result: [String: DiffDocument] = [:] for await (id, doc) in group { - result[id] = doc + guard !Task.isCancelled else { break } + documentCache[id] = doc + if selectedFile?.id == id { + diffDocument = doc + } } - return result } + + guard !Task.isCancelled else { return } + isLoadingFiles = false + + (selectedFile, diffDocument) = Self.resolvedSelection(current: selectedFile, files: files, cache: documentCache) + } + + // MARK: - Live Git integration + + private nonisolated static func liveFetchChangedFiles(worktreeURL: URL) async -> [DiffChangedFile] { + let gitClient = GitClient() + async let trackedOutput = gitClient.diffNameStatus(at: worktreeURL) + async let untrackedPaths = gitClient.untrackedFilePaths(at: worktreeURL) + let trackedFiles = DiffChangedFile.parseNameStatus(await trackedOutput) + let untrackedFiles = await untrackedPaths.map { + DiffChangedFile(status: .added, oldPath: nil, newPath: $0) + } + return trackedFiles + untrackedFiles } - private nonisolated static func loadDocument( + private nonisolated static func liveLoadDocument( for file: DiffChangedFile, worktreeURL: URL ) async -> DiffDocument { diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift new file mode 100644 index 00000000..c4c17a81 --- /dev/null +++ b/supacodeTests/DiffWindowStateTests.swift @@ -0,0 +1,117 @@ +import Foundation +import Testing +import YiTong + +@testable import supacode + +@MainActor +struct DiffWindowStateTests { + @Test func evictedCacheRemovesEntriesNotInFileIDs() { + let cache = [ + "a.swift": DiffDocument(files: [], title: "a"), + "b.swift": DiffDocument(files: [], title: "b"), + ] + let result = DiffWindowState.evictedCache(cache, keeping: ["a.swift"]) + #expect(result.keys.sorted() == ["a.swift"]) + } + + @Test func resolvedSelectionKeepsCurrentWhenItsDocumentIsCached() { + let current = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let other = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let doc = DiffDocument(files: [], title: "a") + let result = DiffWindowState.resolvedSelection( + current: current, + files: [other, current], + cache: ["a.swift": doc] + ) + #expect(result.file == current) + #expect(result.document == doc) + } + + @Test func resolvedSelectionFallsBackToFirstFileWhenCurrentHasNoCachedDocument() { + let current = DiffChangedFile(status: .modified, oldPath: "removed.swift", newPath: "removed.swift") + let first = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let doc = DiffDocument(files: [], title: "a") + let result = DiffWindowState.resolvedSelection( + current: current, + files: [first], + cache: ["a.swift": doc] + ) + #expect(result.file == first) + #expect(result.document == doc) + } + + @Test func resolvedSelectionPicksFirstFileWhenNoneSelected() { + let first = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let second = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let doc = DiffDocument(files: [], title: "a") + let result = DiffWindowState.resolvedSelection( + current: nil, + files: [first, second], + cache: ["a.swift": doc] + ) + #expect(result.file == first) + #expect(result.document == doc) + } + + @Test func resolvedSelectionReturnsNilWhenNoFiles() { + let result = DiffWindowState.resolvedSelection(current: nil, files: [], cache: [:]) + #expect(result.file == nil) + #expect(result.document == nil) + } + + @Test func loadAllFilesPopulatesCacheAndAutoSelectsFirstFile() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docs = [ + "a.swift": DiffDocument(files: [], title: "a"), + "b.swift": DiffDocument(files: [], title: "b"), + ] + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in docs[file.id]! } + ) + + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + + #expect(state.changedFiles == [fileA, fileB]) + #expect(state.selectedFile == fileA) + #expect(state.diffDocument == docs["a.swift"]) + #expect(!state.isLoadingFiles) + } + + @Test func loadAllFilesPreservesSelectionWhenStillPresent() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docs = [ + "a.swift": DiffDocument(files: [], title: "a"), + "b.swift": DiffDocument(files: [], title: "b"), + ] + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in docs[file.id]! } + ) + state.selectedFile = fileB + + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + + #expect(state.selectedFile == fileB) + #expect(state.diffDocument == docs["b.swift"]) + } + + @Test func loadAllFilesClearsSelectionWhenFileRemoved() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let removed = DiffChangedFile(status: .modified, oldPath: "removed.swift", newPath: "removed.swift") + let docs = ["a.swift": DiffDocument(files: [], title: "a")] + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA] }, + loadDiffDocument: { file, _ in docs[file.id]! } + ) + state.selectedFile = removed + + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + + #expect(state.selectedFile == fileA) + #expect(state.diffDocument == docs["a.swift"]) + } +} -- 2.51.2 From 4ad4a5ab63e1b75dd6ec4a59861aea115628537a Mon Sep 17 00:00:00 2001 From: "lenny.cheng" Date: Wed, 1 Jul 2026 15:24:09 +0800 Subject: [PATCH 2/4] feat: Show render-in-progress state and debounce rapid file switches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Show a small centered spinner while the WebView-backed DiffView is still rendering the current document — large files can take 1-2s to diff/paint on the JS side even though the Swift-side cache lookup is instant, so without this the UI looked stuck rather than working. Rapidly flicking through files (A -> B -> C) previously sent a render request for every file passed through, including ones the user only glanced at, causing a visible flash/jump once a skipped-over file's slow render finally completed after the real target had already rendered. selectFile now debounces by 150ms (backed by an injectable Clock, testable via TestClock) so only the selection that's still current once the interval elapses actually triggers a render. - Add `isRenderingDiff` / `markDiffRendered()` to DiffWindowState, routed through a shared `updateDiffDocument` helper - Debounce `selectFile`'s document commit via an injected `Clock`, cancelling the previous pending selection on every new one - Add a centered ProgressView overlay in DiffWindowContentView while `isRenderingDiff` is true - Add tests covering render-state tracking and debounce behavior (pending-state, rapid-switch, cancellation) Files changed: - supacode/Features/DiffView/DiffWindowState.swift - supacode/Features/DiffView/DiffWindowContentView.swift - supacodeTests/DiffWindowStateTests.swift --- .../DiffView/DiffWindowContentView.swift | 15 +++ .../Features/DiffView/DiffWindowState.swift | 50 +++++++- supacodeTests/DiffWindowStateTests.swift | 108 ++++++++++++++++++ 3 files changed, 168 insertions(+), 5 deletions(-) diff --git a/supacode/Features/DiffView/DiffWindowContentView.swift b/supacode/Features/DiffView/DiffWindowContentView.swift index 9caac76e..882b3821 100644 --- a/supacode/Features/DiffView/DiffWindowContentView.swift +++ b/supacode/Features/DiffView/DiffWindowContentView.swift @@ -104,7 +104,22 @@ struct DiffWindowContentView: View { style: diffStyle, showsFileHeaders: false, ), + onEvent: { event in + if case .didRender = event { + state.markDiffRendered() + } + } ) + .overlay { + if state.isRenderingDiff { + ProgressView() + .controlSize(.small) + .padding(12) + .background(.regularMaterial, in: Circle()) + .transition(.opacity) + } + } + .animation(.easeInOut(duration: 0.15), value: state.isRenderingDiff) } 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 cd08dbd2..03a3bd2d 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -10,20 +10,32 @@ final class DiffWindowState { 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 private var documentCache: [String: DiffDocument] = [:] private var loadTask: Task? + private var selectDebounceTask: Task? 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 + = DiffWindowState.liveLoadDocument, + selectDebounceInterval: Duration = .milliseconds(150), + clock: C = ContinuousClock() ) { self.fetchChangedFiles = fetchChangedFiles self.loadDiffDocument = loadDiffDocument + self.selectDebounceInterval = selectDebounceInterval + self.sleep = { duration in try await clock.sleep(for: duration) } } func load(worktreeURL: URL, branchName: String) { @@ -47,7 +59,33 @@ final class DiffWindowState { func selectFile(_ file: DiffChangedFile) { guard selectedFile != file else { return } selectedFile = file - diffDocument = documentCache[file.id] + + // Debounced so that flicking quickly through several files (e.g. A -> B -> C) + // never triggers a render for a file the user only passed through — only the + // selection that's still current once the interval elapses gets applied. + selectDebounceTask?.cancel() + let sleep = self.sleep + let interval = selectDebounceInterval + selectDebounceTask = Task { [weak self, sleep] in + do { + try await sleep(interval) + } catch { + return + } + guard let self, !Task.isCancelled else { return } + self.updateDiffDocument(self.documentCache[file.id]) + } + } + + /// Called by the view once `DiffView` reports its `didRender` event. + func markDiffRendered() { + isRenderingDiff = false + } + + private func updateDiffDocument(_ newDocument: DiffDocument?) { + guard newDocument != diffDocument else { return } + isRenderingDiff = newDocument != nil + diffDocument = newDocument } // MARK: - Reconciliation (pure, testable without hitting Git or Task scheduling) @@ -104,7 +142,7 @@ final class DiffWindowState { guard !Task.isCancelled else { break } documentCache[id] = doc if selectedFile?.id == id { - diffDocument = doc + updateDiffDocument(doc) } } } @@ -112,7 +150,9 @@ final class DiffWindowState { guard !Task.isCancelled else { return } isLoadingFiles = false - (selectedFile, diffDocument) = Self.resolvedSelection(current: selectedFile, files: files, cache: documentCache) + let resolved = Self.resolvedSelection(current: selectedFile, files: files, cache: documentCache) + selectedFile = resolved.file + updateDiffDocument(resolved.document) } // MARK: - Live Git integration diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift index c4c17a81..0d485c0e 100644 --- a/supacodeTests/DiffWindowStateTests.swift +++ b/supacodeTests/DiffWindowStateTests.swift @@ -1,3 +1,4 @@ +import Clocks import Foundation import Testing import YiTong @@ -114,4 +115,111 @@ struct DiffWindowStateTests { #expect(state.selectedFile == fileA) #expect(state.diffDocument == docs["a.swift"]) } + + @Test func selectFileMarksRenderingWhenDocumentIsCached() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in file.id == "a.swift" ? docA : docB }, + clock: clock + ) + // Seed documentCache via the public loading path (auto-selects fileA). + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + await advanceSelectDebounce(clock) + + #expect(state.isRenderingDiff) + #expect(state.diffDocument == docB) + } + + @Test func selectFileDoesNotMarkRenderingWhenDocumentIsUnchanged() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let sharedDoc = DiffDocument(files: [], title: "same") + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { _, _ in sharedDoc }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + await advanceSelectDebounce(clock) + + #expect(!state.isRenderingDiff) + } + + @Test func loadAllFilesMarksRenderingWhenAutoSelectedDocumentArrives() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let docA = DiffDocument(files: [], title: "a") + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA] }, + loadDiffDocument: { _, _ in docA } + ) + + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + + #expect(state.isRenderingDiff) + } + + @Test func selectFileDoesNotUpdateDocumentBeforeDebounceSettles() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in file.id == "a.swift" ? docA : docB }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + await Task.yield() + + #expect(state.diffDocument == docA) + #expect(!state.isRenderingDiff) + } + + @Test func selectFileOnlyAppliesFinalSelectionWhenSwitchedRapidly() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let fileC = DiffChangedFile(status: .modified, oldPath: "c.swift", newPath: "c.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let docC = DiffDocument(files: [], title: "c") + let docs = ["a.swift": docA, "b.swift": docB, "c.swift": docC] + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB, fileC] }, + loadDiffDocument: { file, _ in docs[file.id]! }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + state.selectFile(fileC) + await advanceSelectDebounce(clock) + + #expect(state.selectedFile == fileC) + #expect(state.diffDocument == docC) + } +} + +@MainActor +private func advanceSelectDebounce(_ clock: TestClock, by duration: Duration = .milliseconds(150)) async { + await Task.yield() + await clock.advance(by: duration) + await Task.yield() } -- 2.51.2 From 509e4db484c26c90c4b31bee69bb4b7091d6d6c6 Mon Sep 17 00:00:00 2001 From: "lenny.cheng" Date: Wed, 1 Jul 2026 16:40:11 +0800 Subject: [PATCH 3/4] fix: Guard debounced selection against stale writes after reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on this PR: selectFile's pending debounce task only gets cancelled when routed through selectFile again. If selectedFile changes via a different path in the meantime (e.g. loadAllFiles reconciliation after refresh() or load()), the stale debounce could still fire and overwrite diffDocument/isRenderingDiff for a file that's no longer selected — the same class of stale-write bug this PR already fixed for the load task, just reintroduced on the newer debounce task. - Guard the debounce task's write with selectedFile?.id == file.id, checked right before applying the debounced document - Cancel any pending selectDebounceTask in load(), matching the existing loadTask cancellation there - Add tests for both: a stale debounce after selection changes elsewhere, and load() cancelling a pending debounce --- .../Features/DiffView/DiffWindowState.swift | 5 ++ supacodeTests/DiffWindowStateTests.swift | 51 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/supacode/Features/DiffView/DiffWindowState.swift b/supacode/Features/DiffView/DiffWindowState.swift index 03a3bd2d..f373b402 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -45,6 +45,7 @@ final class DiffWindowState { selectedFile = nil diffDocument = nil documentCache = [:] + selectDebounceTask?.cancel() loadTask?.cancel() loadTask = Task { await loadAllFiles(worktreeURL: worktreeURL) } } @@ -73,6 +74,10 @@ final class DiffWindowState { return } guard let self, !Task.isCancelled 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]) } } diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift index 0d485c0e..b40cfb3a 100644 --- a/supacodeTests/DiffWindowStateTests.swift +++ b/supacodeTests/DiffWindowStateTests.swift @@ -215,6 +215,57 @@ struct DiffWindowStateTests { #expect(state.selectedFile == fileC) #expect(state.diffDocument == docC) } + + @Test func selectFileDebounceSkipsStaleUpdateIfSelectionChangedElsewhere() async { + // Reproduces a review comment on PR onevcat/Prowl#529: a pending debounce + // task only cancels when routed through `selectFile` again. If something + // else (e.g. `loadAllFiles` reconciliation) changes `selectedFile` directly + // in the meantime, the stale debounce must not overwrite state once it fires. + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let fileC = DiffChangedFile(status: .modified, oldPath: "c.swift", newPath: "c.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let docC = DiffDocument(files: [], title: "c") + let docs = ["a.swift": docA, "b.swift": docB, "c.swift": docC] + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB, fileC] }, + loadDiffDocument: { file, _ in docs[file.id]! }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + state.selectedFile = fileC + await advanceSelectDebounce(clock) + + #expect(state.selectedFile == fileC) + #expect(state.diffDocument != docB) + } + + @Test func loadCancelsPendingSelectDebounce() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let docs = ["a.swift": docA, "b.swift": docB] + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in docs[file.id]! }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + + state.selectFile(fileB) + state.load(worktreeURL: URL(fileURLWithPath: "/tmp2"), branchName: "other") + await advanceSelectDebounce(clock) + + #expect(state.diffDocument != docB) + } } @MainActor -- 2.51.2 From 753373e0e90fec98b78f2bf4dbea2b77cf77ae25 Mon Sep 17 00:00:00 2001 From: "lenny.cheng" Date: Wed, 1 Jul 2026 16:45:11 +0800 Subject: [PATCH 4/4] fix: Clear loading indicator and surface an error on DiffView render failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isRenderingDiff was only ever cleared by markDiffRendered() on .didRender — if YiTong's DiffView reports .didFail instead, the loading spinner would stay up forever with no indication anything went wrong. - Add markDiffFailed(_:), clearing isRenderingDiff and storing the DiffError in a new renderError property - Route DiffView's .didFail event to markDiffFailed in DiffWindowContentView, showing a small error message in place of the spinner - updateDiffDocument clears any prior renderError once a new document starts rendering, so switching files (or refreshing) naturally "retries" without a stale error message lingering - Add tests for both paths --- .../DiffView/DiffWindowContentView.swift | 21 +++++++++- .../Features/DiffView/DiffWindowState.swift | 14 +++++++ supacodeTests/DiffWindowStateTests.swift | 39 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/supacode/Features/DiffView/DiffWindowContentView.swift b/supacode/Features/DiffView/DiffWindowContentView.swift index 882b3821..7afeb91e 100644 --- a/supacode/Features/DiffView/DiffWindowContentView.swift +++ b/supacode/Features/DiffView/DiffWindowContentView.swift @@ -105,8 +105,13 @@ struct DiffWindowContentView: View { showsFileHeaders: false, ), onEvent: { event in - if case .didRender = event { + switch event { + case .didRender: state.markDiffRendered() + case .didFail(let error): + state.markDiffFailed(error) + default: + break } } ) @@ -117,9 +122,23 @@ struct DiffWindowContentView: View { .padding(12) .background(.regularMaterial, in: Circle()) .transition(.opacity) + } else if let renderError = state.renderError { + VStack(spacing: 6) { + Image(systemName: "exclamationmark.triangle") + .foregroundStyle(.orange) + .accessibilityHidden(true) + Text(renderError.message) + .font(.caption) + .multilineTextAlignment(.center) + } + .padding(12) + .frame(maxWidth: 240) + .background(.regularMaterial, in: RoundedRectangle(cornerRadius: 8)) + .transition(.opacity) } } .animation(.easeInOut(duration: 0.15), value: state.isRenderingDiff) + .animation(.easeInOut(duration: 0.15), value: state.renderError) } 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 f373b402..49d63689 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -15,6 +15,10 @@ final class DiffWindowState { /// 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? private var documentCache: [String: DiffDocument] = [:] private var loadTask: Task? @@ -87,9 +91,19 @@ final class DiffWindowState { isRenderingDiff = false } + /// 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 + } + private func updateDiffDocument(_ newDocument: DiffDocument?) { guard newDocument != diffDocument else { return } isRenderingDiff = newDocument != nil + if isRenderingDiff { + renderError = nil + } diffDocument = newDocument } diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift index b40cfb3a..aa28c298 100644 --- a/supacodeTests/DiffWindowStateTests.swift +++ b/supacodeTests/DiffWindowStateTests.swift @@ -266,6 +266,45 @@ struct DiffWindowStateTests { #expect(state.diffDocument != docB) } + + @Test func markDiffFailedClearsRenderingAndStoresError() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let docA = DiffDocument(files: [], title: "a") + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA] }, + loadDiffDocument: { _, _ in docA } + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + #expect(state.isRenderingDiff) + + let error = DiffError(code: "render_failed", message: "boom") + state.markDiffFailed(error) + + #expect(!state.isRenderingDiff) + #expect(state.renderError == error) + } + + @Test func selectingANewFileClearsAPriorRenderError() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let fileB = DiffChangedFile(status: .modified, oldPath: "b.swift", newPath: "b.swift") + let docA = DiffDocument(files: [], title: "a") + let docB = DiffDocument(files: [], title: "b") + let docs = ["a.swift": docA, "b.swift": docB] + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA, fileB] }, + loadDiffDocument: { file, _ in docs[file.id]! }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffFailed(DiffError(code: "render_failed", message: "boom")) + #expect(state.renderError != nil) + + state.selectFile(fileB) + await advanceSelectDebounce(clock) + + #expect(state.renderError == nil) + } } @MainActor