From df3c52bb5799e8cb2d3d175f2eef9e14f3a10175 Mon Sep 17 00:00:00 2001 From: onevcat Date: Fri, 3 Jul 2026 23:37:51 +0900 Subject: [PATCH] fix: add render-failure retry path and leading-edge select debounce in diff window Follow-up to #529: - Retrying a failed render now works: refreshing or re-selecting the failed file bumps renderGeneration, which recreates the DiffView via .id() since YiTong skips re-rendering a value-equal document. - selectFile now applies a deliberate selection immediately (leading-edge debounce) instead of always waiting 150ms; only rapid follow-up selections within the window are deferred. - Document the spinner/error overlay and retry behavior in docs. --- docs/components/diff-view.md | 6 +- .../DiffView/DiffWindowContentView.swift | 3 + .../Features/DiffView/DiffWindowState.swift | 40 ++++-- supacodeTests/DiffWindowStateTests.swift | 116 ++++++++++++++++-- 4 files changed, 148 insertions(+), 17 deletions(-) diff --git a/docs/components/diff-view.md b/docs/components/diff-view.md index ba2169e8..8ea94692 100644 --- a/docs/components/diff-view.md +++ b/docs/components/diff-view.md @@ -40,13 +40,17 @@ Tools that are not installed on the Mac are shown disabled in the Diff Tool menu - The selected file's diff, comparing the **HEAD** version (`git show HEAD:path`) against the **on-disk** version. - Both tracked changes and **untracked new files** are included. +- A small **spinner** overlays the diff while a large file is still rendering, + and an **error overlay** appears if rendering fails. ## Modes & interactions - **Split** (side-by-side, default) or **Unified** view — toggle via the toolbar picker. -- Click a file in the list to view its diff. +- Click a file in the list to view its diff. Rapid switching is debounced: the + first selection renders immediately, files flicked through are skipped. - Auto-refresh on focus keeps it current as the agent keeps working. +- If a render fails, **re-selecting the file** (or any refresh) retries it. ## Line-change badges elsewhere diff --git a/supacode/Features/DiffView/DiffWindowContentView.swift b/supacode/Features/DiffView/DiffWindowContentView.swift index 7afeb91e..23d17a8b 100644 --- a/supacode/Features/DiffView/DiffWindowContentView.swift +++ b/supacode/Features/DiffView/DiffWindowContentView.swift @@ -115,6 +115,9 @@ struct DiffWindowContentView: View { } } ) + // YiTong skips re-rendering a value-equal document, so retrying after a + // render failure works by recreating the view with a new identity. + .id(state.renderGeneration) .overlay { if state.isRenderingDiff { ProgressView() diff --git a/supacode/Features/DiffView/DiffWindowState.swift b/supacode/Features/DiffView/DiffWindowState.swift index 49d63689..d52a6f0f 100644 --- a/supacode/Features/DiffView/DiffWindowState.swift +++ b/supacode/Features/DiffView/DiffWindowState.swift @@ -19,6 +19,11 @@ final class DiffWindowState { /// the render-in-progress indicator doesn't stay stuck forever. Cleared as soon /// as a new document starts rendering. var renderError: DiffError? + /// 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 + /// on retry (refresh or re-selecting the failed file) does exactly that. + private(set) var renderGeneration = 0 private var documentCache: [String: DiffDocument] = [:] private var loadTask: Task? @@ -31,8 +36,8 @@ final class DiffWindowState { init>( fetchChangedFiles: @escaping @Sendable (URL) async -> [DiffChangedFile] = DiffWindowState.liveFetchChangedFiles, - loadDiffDocument: @escaping @Sendable (DiffChangedFile, URL) async -> DiffDocument - = DiffWindowState.liveLoadDocument, + loadDiffDocument: @escaping @Sendable (DiffChangedFile, URL) async -> DiffDocument = DiffWindowState + .liveLoadDocument, selectDebounceInterval: Duration = .milliseconds(150), clock: C = ContinuousClock() ) { @@ -50,6 +55,7 @@ final class DiffWindowState { diffDocument = nil documentCache = [:] selectDebounceTask?.cancel() + selectDebounceTask = nil loadTask?.cancel() loadTask = Task { await loadAllFiles(worktreeURL: worktreeURL) } } @@ -62,12 +68,19 @@ final class DiffWindowState { } func selectFile(_ file: DiffChangedFile) { - guard selectedFile != file else { return } + // 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 } selectedFile = file - // 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. + // 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 { + updateDiffDocument(documentCache[file.id]) + } selectDebounceTask?.cancel() let sleep = self.sleep let interval = selectDebounceInterval @@ -78,6 +91,9 @@ final class DiffWindowState { return } 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. @@ -99,7 +115,17 @@ final class DiffWindowState { } private func updateDiffDocument(_ newDocument: DiffDocument?) { - guard newDocument != diffDocument else { return } + if newDocument == diffDocument { + // Re-applying an equal document is normally a no-op, but after a render + // 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 + renderGeneration += 1 + return + } isRenderingDiff = newDocument != nil if isRenderingDiff { renderError = nil diff --git a/supacodeTests/DiffWindowStateTests.swift b/supacodeTests/DiffWindowStateTests.swift index aa28c298..24a6441c 100644 --- a/supacodeTests/DiffWindowStateTests.swift +++ b/supacodeTests/DiffWindowStateTests.swift @@ -170,7 +170,9 @@ struct DiffWindowStateTests { #expect(state.isRenderingDiff) } - @Test func selectFileDoesNotUpdateDocumentBeforeDebounceSettles() async { + @Test func selectFileAppliesCachedDocumentImmediately() async { + // Leading edge of the debounce: a deliberate single selection must not wait + // out the debounce interval when its document is already cached. 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") @@ -185,10 +187,37 @@ struct DiffWindowStateTests { state.markDiffRendered() state.selectFile(fileB) + + #expect(state.diffDocument == docB) + #expect(state.isRenderingDiff) + } + + @Test func selectFileDefersFollowUpSelectionWithinDebounceWindow() 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 Task.yield() - #expect(state.diffDocument == docA) - #expect(!state.isRenderingDiff) + #expect(state.diffDocument == docB) + + await advanceSelectDebounce(clock) + + #expect(state.diffDocument == docC) } @Test func selectFileOnlyAppliesFinalSelectionWhenSwitchedRapidly() async { @@ -238,22 +267,26 @@ struct DiffWindowStateTests { state.markDiffRendered() state.selectFile(fileB) - state.selectedFile = fileC + state.selectFile(fileC) + state.selectedFile = fileA await advanceSelectDebounce(clock) - #expect(state.selectedFile == fileC) - #expect(state.diffDocument != docB) + #expect(state.selectedFile == fileA) + #expect(state.diffDocument == docB) + #expect(state.diffDocument != docC) } @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 fileC = DiffChangedFile(status: .modified, oldPath: "c.swift", newPath: "c.swift") let docA = DiffDocument(files: [], title: "a") let docB = DiffDocument(files: [], title: "b") - let docs = ["a.swift": docA, "b.swift": docB] + 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] }, + fetchChangedFiles: { _ in [fileA, fileB, fileC] }, loadDiffDocument: { file, _ in docs[file.id]! }, clock: clock ) @@ -261,10 +294,11 @@ struct DiffWindowStateTests { state.markDiffRendered() state.selectFile(fileB) + state.selectFile(fileC) state.load(worktreeURL: URL(fileURLWithPath: "/tmp2"), branchName: "other") await advanceSelectDebounce(clock) - #expect(state.diffDocument != docB) + #expect(state.diffDocument != docC) } @Test func markDiffFailedClearsRenderingAndStoresError() async { @@ -305,6 +339,70 @@ struct DiffWindowStateTests { #expect(state.renderError == nil) } + + @Test func reselectingFailedFileRetriesRender() async { + // YiTong skips re-rendering a value-equal document, so a retry must bump + // `renderGeneration` to recreate the view instead of re-applying the doc. + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let docA = DiffDocument(files: [], title: "a") + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA] }, + loadDiffDocument: { _, _ in docA }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffFailed(DiffError(code: "render_failed", message: "boom")) + let generationBefore = state.renderGeneration + + state.selectFile(fileA) + + #expect(state.renderError == nil) + #expect(state.isRenderingDiff) + #expect(state.renderGeneration == generationBefore + 1) + #expect(state.diffDocument == docA) + } + + @Test func refreshRetriesFailedRenderOfUnchangedDocument() 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")) + state.markDiffRendered() + state.markDiffFailed(DiffError(code: "render_failed", message: "boom")) + let generationBefore = state.renderGeneration + + // Drive the reload directly, as `refresh()` would; the file's content is + // 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.renderGeneration == generationBefore + 1) + #expect(state.diffDocument == docA) + } + + @Test func reselectingSameFileWithoutErrorIsANoOp() async { + let fileA = DiffChangedFile(status: .modified, oldPath: "a.swift", newPath: "a.swift") + let docA = DiffDocument(files: [], title: "a") + let clock = TestClock() + let state = DiffWindowState( + fetchChangedFiles: { _ in [fileA] }, + loadDiffDocument: { _, _ in docA }, + clock: clock + ) + await state.loadAllFiles(worktreeURL: URL(fileURLWithPath: "/tmp")) + state.markDiffRendered() + let generationBefore = state.renderGeneration + + state.selectFile(fileA) + + #expect(!state.isRenderingDiff) + #expect(state.renderGeneration == generationBefore) + } } @MainActor -- 2.51.2