From 7f1bc0d07ba7764adbd701d13c998b58b5de62a8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:26:44 +1000 Subject: [PATCH 1/4] fix(swift-ios): retain review content during refresh failures --- .../Features/Review/FeatureReviewView.swift | 64 ++++++++++++++++--- .../Shared/FeatureRefreshFailureRow.swift | 20 ++++++ 2 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift diff --git a/apps/swift-ios/Features/Review/FeatureReviewView.swift b/apps/swift-ios/Features/Review/FeatureReviewView.swift index 0cf291b32555f..edfd81c351d6d 100644 --- a/apps/swift-ios/Features/Review/FeatureReviewView.swift +++ b/apps/swift-ios/Features/Review/FeatureReviewView.swift @@ -12,6 +12,7 @@ public struct FeatureReviewView: View { @State private var review: FeatureReview? @State private var isLoading = true @State private var errorMessage: String? + @State private var loadGeneration = FeatureAsyncGeneration() public init( client: any FeatureClient, @@ -60,6 +61,14 @@ public struct FeatureReviewView: View { private func reviewList(_ review: FeatureReview) -> some View { List { + if let errorMessage { + Section { + FeatureRefreshFailureRow(message: errorMessage) { + Task { await load() } + } + } + } + Section { HStack { VStack(alignment: .leading, spacing: 3) { @@ -112,12 +121,18 @@ public struct FeatureReviewView: View { } private func load() async { + let generation = loadGeneration.begin() isLoading = true - defer { isLoading = false } + defer { + if loadGeneration.accepts(generation) { isLoading = false } + } do { - review = try await client.loadReview(threadID: threadID) + let loaded = try await client.loadReview(threadID: threadID) + guard loadGeneration.accepts(generation) else { return } + review = loaded errorMessage = nil } catch { + guard loadGeneration.accepts(generation) else { return } errorMessage = error.localizedDescription } } @@ -206,6 +221,8 @@ private struct FeatureDiffView: View { @State private var renderedLines: [FeatureDiffLine] @State private var isHydrating = false + @State private var hydrationError: String? + @State private var hydrationGeneration = FeatureAsyncGeneration() @State private var selectedLine: FeatureReviewLineSelection? @State private var isCommenting = false @State private var comment = "" @@ -231,6 +248,15 @@ private struct FeatureDiffView: View { if renderedLines.isEmpty, isHydrating { ProgressView("Loading full diff…") .frame(maxWidth: .infinity, maxHeight: .infinity) + } else if let hydrationError, renderedLines.isEmpty { + ContentUnavailableView { + Label("Couldn’t load this diff", systemImage: "exclamationmark.circle") + } description: { + Text(hydrationError) + } actions: { + Button("Try again") { Task { await hydrate() } } + .buttonStyle(.borderedProminent) + } } else if renderedLines.isEmpty { ContentUnavailableView( file.change == .binary ? "Binary file" : "Diff unavailable", @@ -278,6 +304,16 @@ private struct FeatureDiffView: View { commentComposer } } + .safeAreaInset(edge: .top, spacing: 0) { + if let hydrationError, !renderedLines.isEmpty { + FeatureRefreshFailureRow(message: hydrationError) { + Task { await hydrate() } + } + .padding(.horizontal, 14) + .padding(.vertical, 10) + .background(T3Colors.surface) + } + } .task(id: file.id) { await hydrate() } } @@ -409,15 +445,27 @@ private struct FeatureDiffView: View { } private func hydrate() async { + let generation = hydrationGeneration.begin() + hydrationError = nil isHydrating = true - defer { isHydrating = false } - guard let contents = try? await client.loadReviewFileContents( - threadID: threadID, - file: file - ) else { + defer { + if hydrationGeneration.accepts(generation) { isHydrating = false } + } + do { + guard let contents = try await client.loadReviewFileContents( + threadID: threadID, + file: file + ) else { + return + } + guard hydrationGeneration.accepts(generation) else { return } + renderedLines = FeatureFullDiffHydrator.lines(for: file, contents: contents) + } catch is CancellationError { return + } catch { + guard hydrationGeneration.accepts(generation) else { return } + hydrationError = error.localizedDescription } - renderedLines = FeatureFullDiffHydrator.lines(for: file, contents: contents) } private func sendComment() { diff --git a/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift b/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift new file mode 100644 index 0000000000000..76bb7d0fe0f49 --- /dev/null +++ b/apps/swift-ios/Features/Shared/FeatureRefreshFailureRow.swift @@ -0,0 +1,20 @@ +import SwiftUI + +struct FeatureRefreshFailureRow: View { + let message: String + let retry: () -> Void + + var body: some View { + HStack(alignment: .top, spacing: 10) { + Image(systemName: "exclamationmark.circle") + .foregroundStyle(T3Colors.warning) + Text(message) + .font(T3Typography.supporting) + .foregroundStyle(T3Colors.textSecondary) + .frame(maxWidth: .infinity, alignment: .leading) + Button("Retry", action: retry) + .font(T3Typography.control.weight(.semibold)) + } + .accessibilityElement(children: .contain) + } +} From 942d68ccce9bdeac061db214a4502ac2d2d8b764 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 22:42:45 +1000 Subject: [PATCH 2/4] test(swift-ios): retain focused review refresh coverage --- .../Features/Shared/FeatureAsyncGeneration.swift | 12 ++++++++++++ .../FeatureAsyncGenerationTests.swift | 15 +++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift create mode 100644 apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift diff --git a/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift b/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift new file mode 100644 index 0000000000000..667e374f72585 --- /dev/null +++ b/apps/swift-ios/Features/Shared/FeatureAsyncGeneration.swift @@ -0,0 +1,12 @@ +struct FeatureAsyncGeneration { + private var value: UInt64 = 0 + + mutating func begin() -> UInt64 { + value &+= 1 + return value + } + + func accepts(_ generation: UInt64) -> Bool { + generation == value + } +} diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift new file mode 100644 index 0000000000000..4e250eb1f61f8 --- /dev/null +++ b/apps/swift-ios/Tests/FeatureTests/FeatureAsyncGenerationTests.swift @@ -0,0 +1,15 @@ +import Testing +@testable import T3Code + +struct FeatureAsyncGenerationTests { + @Test + func asyncGenerationRejectsAnOlderCompletionAfterANewerOperationBegins() { + var generation = FeatureAsyncGeneration() + let staleLoad = generation.begin() + let mutation = generation.begin() + + #expect(!generation.accepts(staleLoad)) + #expect(generation.accepts(mutation)) + } + +} From d648bf99c1ec4ab93a9ffeea3efcddcff0aff621 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:25:21 +1000 Subject: [PATCH 3/4] test(swift-ios): cover diff hydration failure and retry states Keep FeatureDiffView's full-file hydration state in one small value so the retained-lines, empty-diff error, Retry, and stale-completion rules have focused tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Review/FeatureDiffHydration.swift | 37 +++++++++ .../Features/Review/FeatureReviewView.swift | 44 ++++------- .../FeatureDiffHydrationTests.swift | 75 +++++++++++++++++++ 3 files changed, 128 insertions(+), 28 deletions(-) create mode 100644 apps/swift-ios/Features/Review/FeatureDiffHydration.swift create mode 100644 apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift diff --git a/apps/swift-ios/Features/Review/FeatureDiffHydration.swift b/apps/swift-ios/Features/Review/FeatureDiffHydration.swift new file mode 100644 index 0000000000000..bf54571ee0e3d --- /dev/null +++ b/apps/swift-ios/Features/Review/FeatureDiffHydration.swift @@ -0,0 +1,37 @@ +/// Lines shown by a review diff while its full file contents load. A failed +/// load keeps the lines already on screen and reports the error for Retry; +/// a completion from a superseded attempt is ignored. +struct FeatureDiffHydration { + private(set) var lines: [FeatureDiffLine] + private(set) var isLoading = false + private(set) var errorMessage: String? + private var generation = FeatureAsyncGeneration() + + init(lines: [FeatureDiffLine]) { + self.lines = lines + } + + mutating func begin() -> UInt64 { + errorMessage = nil + isLoading = true + return generation.begin() + } + + /// `nil` means the server has no full contents, so the patch lines stay. + mutating func succeed(_ attempt: UInt64, lines loaded: [FeatureDiffLine]?) { + guard generation.accepts(attempt) else { return } + if let loaded { lines = loaded } + isLoading = false + } + + mutating func fail(_ attempt: UInt64, message: String) { + guard generation.accepts(attempt) else { return } + errorMessage = message + isLoading = false + } + + mutating func cancel(_ attempt: UInt64) { + guard generation.accepts(attempt) else { return } + isLoading = false + } +} diff --git a/apps/swift-ios/Features/Review/FeatureReviewView.swift b/apps/swift-ios/Features/Review/FeatureReviewView.swift index edfd81c351d6d..7d8d131beb355 100644 --- a/apps/swift-ios/Features/Review/FeatureReviewView.swift +++ b/apps/swift-ios/Features/Review/FeatureReviewView.swift @@ -219,10 +219,7 @@ private struct FeatureDiffView: View { let file: FeatureReviewFile let sendMessage: (FeatureMessageSubmission) async -> Bool - @State private var renderedLines: [FeatureDiffLine] - @State private var isHydrating = false - @State private var hydrationError: String? - @State private var hydrationGeneration = FeatureAsyncGeneration() + @State private var hydration: FeatureDiffHydration @State private var selectedLine: FeatureReviewLineSelection? @State private var isCommenting = false @State private var comment = "" @@ -240,15 +237,15 @@ private struct FeatureDiffView: View { self.threadID = threadID self.file = file self.sendMessage = sendMessage - _renderedLines = State(initialValue: file.lines) + _hydration = State(initialValue: FeatureDiffHydration(lines: file.lines)) } var body: some View { Group { - if renderedLines.isEmpty, isHydrating { + if hydration.lines.isEmpty, hydration.isLoading { ProgressView("Loading full diff…") .frame(maxWidth: .infinity, maxHeight: .infinity) - } else if let hydrationError, renderedLines.isEmpty { + } else if let hydrationError = hydration.errorMessage, hydration.lines.isEmpty { ContentUnavailableView { Label("Couldn’t load this diff", systemImage: "exclamationmark.circle") } description: { @@ -257,7 +254,7 @@ private struct FeatureDiffView: View { Button("Try again") { Task { await hydrate() } } .buttonStyle(.borderedProminent) } - } else if renderedLines.isEmpty { + } else if hydration.lines.isEmpty { ContentUnavailableView( file.change == .binary ? "Binary file" : "Diff unavailable", systemImage: file.change == .binary ? "doc.richtext" : "doc.text.magnifyingglass", @@ -267,7 +264,7 @@ private struct FeatureDiffView: View { GeometryReader { proxy in ScrollView([.horizontal, .vertical]) { LazyVStack(alignment: .leading, spacing: 0) { - ForEach(renderedLines) { line in + ForEach(hydration.lines) { line in FeatureDiffLineRow( line: line, isSelected: selection(for: line) == selectedLine, @@ -305,7 +302,7 @@ private struct FeatureDiffView: View { } } .safeAreaInset(edge: .top, spacing: 0) { - if let hydrationError, !renderedLines.isEmpty { + if let hydrationError = hydration.errorMessage, !hydration.lines.isEmpty { FeatureRefreshFailureRow(message: hydrationError) { Task { await hydrate() } } @@ -445,32 +442,23 @@ private struct FeatureDiffView: View { } private func hydrate() async { - let generation = hydrationGeneration.begin() - hydrationError = nil - isHydrating = true - defer { - if hydrationGeneration.accepts(generation) { isHydrating = false } - } + let attempt = hydration.begin() do { - guard let contents = try await client.loadReviewFileContents( - threadID: threadID, - file: file - ) else { - return - } - guard hydrationGeneration.accepts(generation) else { return } - renderedLines = FeatureFullDiffHydrator.lines(for: file, contents: contents) + let contents = try await client.loadReviewFileContents(threadID: threadID, file: file) + hydration.succeed( + attempt, + lines: contents.map { FeatureFullDiffHydrator.lines(for: file, contents: $0) } + ) } catch is CancellationError { - return + hydration.cancel(attempt) } catch { - guard hydrationGeneration.accepts(generation) else { return } - hydrationError = error.localizedDescription + hydration.fail(attempt, message: error.localizedDescription) } } private func sendComment() { guard !trimmedComment.isEmpty, !isSending else { return } - let record = reviewDraft.contextRecord(lines: renderedLines) + let record = reviewDraft.contextRecord(lines: hydration.lines) let prompt = reviewDraft.submissionText(contextRecord: record) isSending = true commentError = nil diff --git a/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift b/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift new file mode 100644 index 0000000000000..032e81b3a9fc3 --- /dev/null +++ b/apps/swift-ios/Tests/FeatureTests/FeatureDiffHydrationTests.swift @@ -0,0 +1,75 @@ +import Testing +@testable import T3Code + +struct FeatureDiffHydrationTests { + private let patch = [ + FeatureDiffLine(id: "patch-1", kind: .deletion, oldLine: 2, text: "let color = blue"), + FeatureDiffLine(id: "patch-2", kind: .addition, newLine: 2, text: "let color = green"), + ] + private let full = [ + FeatureDiffLine(id: "full-1", kind: .context, oldLine: 1, newLine: 1, text: "import SwiftUI"), + FeatureDiffLine(id: "full-2", kind: .deletion, oldLine: 2, text: "let color = blue"), + FeatureDiffLine(id: "full-3", kind: .addition, newLine: 2, text: "let color = green"), + ] + + @Test + func failedLoadKeepsPatchLinesUntilRetrySucceeds() { + var hydration = FeatureDiffHydration(lines: patch) + + let failed = hydration.begin() + hydration.fail(failed, message: "The network connection was lost.") + + #expect(hydration.lines == patch) + #expect(hydration.errorMessage == "The network connection was lost.") + #expect(!hydration.isLoading) + + let retry = hydration.begin() + #expect(hydration.errorMessage == nil) + #expect(hydration.isLoading) + + hydration.succeed(retry, lines: full) + + #expect(hydration.lines == full) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } + + @Test + func failedLoadWithoutPatchLinesReportsTheError() { + var hydration = FeatureDiffHydration(lines: []) + + let failed = hydration.begin() + hydration.fail(failed, message: "Request timed out.") + + #expect(hydration.lines.isEmpty) + #expect(hydration.errorMessage == "Request timed out.") + #expect(!hydration.isLoading) + } + + @Test + func supersededAttemptCannotOverwriteNewerResult() { + var hydration = FeatureDiffHydration(lines: patch) + + let stale = hydration.begin() + let current = hydration.begin() + hydration.succeed(current, lines: full) + hydration.fail(stale, message: "The request was cancelled.") + hydration.succeed(stale, lines: []) + + #expect(hydration.lines == full) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } + + @Test + func missingFullContentsKeepPatchLinesWithoutError() { + var hydration = FeatureDiffHydration(lines: patch) + + let attempt = hydration.begin() + hydration.succeed(attempt, lines: nil) + + #expect(hydration.lines == patch) + #expect(hydration.errorMessage == nil) + #expect(!hydration.isLoading) + } +} From badb34dfe07cc8599d151f2e44dd0e29507bd228 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 22:25:21 +1000 Subject: [PATCH 4/4] fix(swift-ios): ignore cancelled review refreshes A cancelled review load is not a failure, so it no longer shows the refresh error row. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/swift-ios/Features/Review/FeatureReviewView.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/swift-ios/Features/Review/FeatureReviewView.swift b/apps/swift-ios/Features/Review/FeatureReviewView.swift index 7d8d131beb355..9aad5b2437012 100644 --- a/apps/swift-ios/Features/Review/FeatureReviewView.swift +++ b/apps/swift-ios/Features/Review/FeatureReviewView.swift @@ -131,6 +131,8 @@ public struct FeatureReviewView: View { guard loadGeneration.accepts(generation) else { return } review = loaded errorMessage = nil + } catch is CancellationError { + return } catch { guard loadGeneration.accepts(generation) else { return } errorMessage = error.localizedDescription