diff --git a/Packages/Sources/DiffView/DiffComputation.swift b/Packages/Sources/DiffView/DiffComputation.swift index 9c3667ca..82d735c5 100644 --- a/Packages/Sources/DiffView/DiffComputation.swift +++ b/Packages/Sources/DiffView/DiffComputation.swift @@ -69,6 +69,37 @@ public enum DiffComputation { return computeUnifiedDiffLines(old: originalLines, new: currentLines) } + // MARK: - Thread edit: snapshot pair with hunk fallback + + /// Picks the best in-memory diff for a thread-edited file using only the + /// snapshots and hunks the thread already recorded. Prefers the exact + /// snapshot-pair diff (`buildSnapshotDiffLines`). When that pair collapses + /// — e.g. the pre-edit snapshot was captured *after* the edit applied so + /// `originalContent == modifiedContent` — falls back to rendering the + /// recorded hunks via `buildEditDiffLines`, so real edits never silently + /// vanish into a "No changes" body when the sidebar is reporting non-zero + /// `+N/-N` from the same hunks. + public nonisolated static func buildThreadEditDiff( + originalContent: String?, + modifiedContent: String?, + hunks: [PreviewFile.EditHunk] + ) -> [DiffLine] { + if let modifiedContent { + let snapshotLines = buildSnapshotDiffLines( + original: originalContent ?? "", + current: modifiedContent + ) + // A collapsed snapshot pair (original == modified) still produces + // a full list of context lines — that's "no real change", so + // require at least one +/- line before trusting the snapshot diff. + let hasRealChange = snapshotLines.contains { $0.kind == .added || $0.kind == .removed } + if hasRealChange { + return snapshotLines + } + } + return buildEditDiffLines(from: hunks) + } + // MARK: - Hunks reverse-applied to current → DiffLine /// Renders the full current file with the supplied hunks highlighted as diff --git a/Packages/Sources/RxCodeChatKit/FileDiffView.swift b/Packages/Sources/RxCodeChatKit/FileDiffView.swift index af4fb516..d2ff9a3b 100644 --- a/Packages/Sources/RxCodeChatKit/FileDiffView.swift +++ b/Packages/Sources/RxCodeChatKit/FileDiffView.swift @@ -247,29 +247,41 @@ public struct FileDiffView: View { // captured from disk at the right moments (pre-first-edit and // post-each-edit). Diff them directly — no view-time disk read, no // contamination from external concurrent edits on the same file. - if showFullFileDiff, let modified = modifiedContent { - let original = originalContent ?? "" + // When the pair collapses (original == modified, e.g. a capture-time + // race) the helper falls back to the recorded hunks so real edits + // never render as "No changes". + if showFullFileDiff, modifiedContent != nil { + let original = originalContent + let modified = modifiedContent + let hunks = editHunks let lines = await Task.detached(priority: .userInitiated) { - DiffComputation.buildSnapshotDiffLines(original: original, current: modified) + DiffComputation.buildThreadEditDiff( + originalContent: original, + modifiedContent: modified, + hunks: hunks + ) }.value if !lines.isEmpty { diffLines = lines return } - // Snapshot pair collapsed (e.g. original lost its capture race). - // Fall through to the legacy / hunk paths so a real edit still - // shows something instead of an empty "no changes" state. } // Legacy fallback for rows persisted before modifiedContent existed: - // diff the original snapshot against the current on-disk content. + // diff the original snapshot against the current on-disk content. If + // disk happens to match the captured original (snapshot-capture race), + // fall through to the hunk path so the recorded edits still render. if showFullFileDiff, let original = originalContent { let path = filePath - diffLines = await Task.detached(priority: .userInitiated) { + let lines = await Task.detached(priority: .userInitiated) { let current = (try? String(contentsOfFile: path, encoding: .utf8)) ?? "" return DiffComputation.buildSnapshotDiffLines(original: original, current: current) }.value - return + let hasRealChange = lines.contains { $0.kind == .added || $0.kind == .removed } + if hasRealChange { + diffLines = lines + return + } } if !editHunks.isEmpty { diff --git a/Packages/Tests/DiffViewTests/DiffComputationTests.swift b/Packages/Tests/DiffViewTests/DiffComputationTests.swift index e4b49300..37f45930 100644 --- a/Packages/Tests/DiffViewTests/DiffComputationTests.swift +++ b/Packages/Tests/DiffViewTests/DiffComputationTests.swift @@ -204,6 +204,141 @@ struct DiffComputationTests { ]) } + // MARK: - Edge cases pulled from real chat threads + + /// Case 1 (AppState+MobileSync.swift on fix/mobilesync-fanout-perf): + /// thread recorded a large pure-deletion edit (199 lines removed); git diff + /// matches. Snapshot-pair captured a race where `originalContent` and + /// `modifiedContent` ended up identical (both post-edit). Sidebar fell + /// back to hunk counts and showed `-200 +1`, but the body short-circuited + /// to "No changes" because the snapshot-pair diff was empty and the + /// legacy disk-read fallback returned unconditionally with empty lines. + /// `buildThreadEditDiff` must fall back to the hunk-based diff so the + /// body still renders the recorded edit. + @Test("thread edit diff falls back to hunks when snapshot pair collapses") + func threadEditDiffFallsBackWhenSnapshotPairCollapses() { + // Mirrors the AppState+MobileSync.swift case: one hunk replaces a + // 200-line block with 1 line. Snapshots collapsed identical so the + // pair diff would produce only context lines — the helper must + // recognise that as "no real change" and render the hunk instead. + let postEdit = "line A\nline B\nline C\n" + let deletedBlock = (1...200).map { "deleted line \($0)" }.joined(separator: "\n") + let hunk = PreviewFile.EditHunk(oldString: deletedBlock, newString: "kept") + + let lines = DiffComputation.buildThreadEditDiff( + originalContent: postEdit, + modifiedContent: postEdit, + hunks: [hunk] + ) + + let removed = lines.filter { $0.kind == .removed }.count + let added = lines.filter { $0.kind == .added }.count + let context = lines.filter { $0.kind == .context }.count + #expect(removed == 200) + #expect(added == 1) + #expect(context == 0, "Collapsed-pair fallback should render the hunk, not all-context lines from the equal snapshots") + } + + @Test("thread edit diff treats an all-context snapshot pair as no real change") + func threadEditDiffTreatsAllContextAsNoChange() { + // Even a non-empty buildSnapshotDiffLines result counts as "no + // change" when every line is context — that's the symptom from the + // production AppState+MobileSync.swift bug where the body listed the + // file but the change counter said "No changes". + let snapshot = "alpha\nbeta\ngamma\n" + let snapshotLines = DiffComputation.buildSnapshotDiffLines(original: snapshot, current: snapshot) + #expect(!snapshotLines.isEmpty) + #expect(snapshotLines.allSatisfy { $0.kind == .context }) + + let hunk = PreviewFile.EditHunk(oldString: "removed", newString: "") + let lines = DiffComputation.buildThreadEditDiff( + originalContent: snapshot, + modifiedContent: snapshot, + hunks: [hunk] + ) + #expect(lines.map(\.text) == ["-removed"]) + } + + /// Case 2 (new-file `Write` row on fix/mobilesync-fanout-perf): the + /// thread recorded a `Write` tool call creating a file with ~201 lines — + /// the orange "+" sidebar badge in ThreadChangesSheet that fires on + /// `containsWrite`. Snapshot capture lost its race so `originalContent` + /// and `modifiedContent` both ended up populated with the post-write + /// content (instead of `originalContent` being `nil` / `""`). The + /// sidebar's `turnStat` saw `snapshotStat == (0, 0)` from the collapsed + /// pair, fell back to `hunkStat` and counted `newString` newlines — + /// reporting `+201` — but the detail view rendered only context lines + /// because `buildSnapshotDiffLines(same, same)` is pure-context. Result + /// on screen: row shows `+201`, body shows zero added rows. The diff + /// computation must fall back to the recorded `("", newString)` hunk so + /// the body matches the sidebar's `+201`, with pure-create gutter + /// numbering 1…N on the new side and a `nil` old gutter throughout. + /// Counts/content here are real-shape (a 201-line file write) rather + /// than literal bytes from the branch — git remains the source of + /// truth for the *branch* this scenario was captured on, the test + /// fixes the *contract* the helper must honour for any such row. + @Test("thread edit diff falls back to hunks for new-file Write when snapshot pair collapses") + func threadEditDiffFallsBackForNewFileWriteWhenSnapshotPairCollapses() { + let writtenContent = (1...201).map { "line \($0)" }.joined(separator: "\n") + "\n" + let hunk = PreviewFile.EditHunk(oldString: "", newString: writtenContent) + + let lines = DiffComputation.buildThreadEditDiff( + originalContent: writtenContent, + modifiedContent: writtenContent, + hunks: [hunk] + ) + + let added = lines.filter { $0.kind == .added }.count + let removed = lines.filter { $0.kind == .removed }.count + let context = lines.filter { $0.kind == .context }.count + #expect(added == 201) + #expect(removed == 0) + #expect( + context == 0, + "Collapsed-pair fallback for a new-file Write must render the hunk, not 201 context lines from the equal snapshots" + ) + // Pure-create numbering: new gutter 1…201, old gutter empty. + #expect(lines.compactMap(\.newLineNumber) == Array(1...201)) + #expect(lines.allSatisfy { $0.oldLineNumber == nil }) + } + + @Test("thread edit diff prefers snapshot pair when both differ") + func threadEditDiffPrefersSnapshotPair() { + let hunk = PreviewFile.EditHunk(oldString: "two", newString: "TWO") + let lines = DiffComputation.buildThreadEditDiff( + originalContent: "one\ntwo\nthree\n", + modifiedContent: "one\nTWO\nthree\n", + hunks: [hunk] + ) + + #expect(lines.map(\.text) == [" one", "-two", "+TWO", " three"]) + #expect(lines.map(\.oldLineNumber) == [1, 2, nil, 3]) + #expect(lines.map(\.newLineNumber) == [1, nil, 2, 3]) + } + + @Test("thread edit diff falls back to hunks when modifiedContent is missing") + func threadEditDiffFallsBackWhenModifiedContentMissing() { + let hunk = PreviewFile.EditHunk(oldString: "foo", newString: "FOO") + let lines = DiffComputation.buildThreadEditDiff( + originalContent: "foo\n", + modifiedContent: nil, + hunks: [hunk] + ) + + #expect(lines.map(\.text) == ["-foo", "+FOO"]) + } + + @Test("thread edit diff returns empty when there is no snapshot pair and no hunks") + func threadEditDiffReturnsEmptyWhenNothingRecorded() { + let lines = DiffComputation.buildThreadEditDiff( + originalContent: nil, + modifiedContent: nil, + hunks: [] + ) + + #expect(lines.isEmpty) + } + @Test("unified diff parser tracks gutter line numbers from hunk header") func unifiedDiffParserTracksLineNumbers() { let raw = """