Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions Packages/Sources/DiffView/DiffComputation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 21 additions & 9 deletions Packages/Sources/RxCodeChatKit/FileDiffView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
135 changes: 135 additions & 0 deletions Packages/Tests/DiffViewTests/DiffComputationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """
Expand Down
Loading