perf(diff): commit-id-keyed LRU diff cache

Key the diff cache by the immutable commit id (falling back to rev) and
fold ignoreWhitespace into the key, so amended/rebased changes and
whitespace toggles never serve a stale diff. Bound DiffCache as a
byte-budgeted LRU and drop the per-selection clear() so previously viewed
changes stay warm for instant back-navigation. Adds DiffCacheTests.

Author: hewigovens

shell/mac/Sources/JayJay/Detail/DetailHeader.swift

@@ -123,10 +123,11 @@ extension ChangeDetailView {
         loadTrackedGitLfsPaths()
         loadDiffStats()
         refreshReviewedPaths()
-        diffStore.clear()
+        // No clear(): content-addressed by commit id, so prior changes stay warm and never go stale.
         diffStore.preload(
             hunks: detail.diff,
             rev: detailRevision,
+            commitId: detail.info.commitId,
             repo: repo,
             compareFromRev: compareFromId,
             ignoreWhitespace: appSettings.ignoreWhitespace

shell/mac/Sources/JayJay/Detail/DetailView.swift

@@ -288,6 +288,7 @@ struct ChangeDetailView: View {
                     DiffSection(
                         hunk: hunk,
                         rev: detailRevision,
+                        commitId: detail.info.commitId,
                         repo: repo,
                         actions: actions,
                         isWorkingCopy: detail.info.isWorkingCopy,

shell/mac/Sources/JayJay/Diff/DiffSection.swift

@@ -5,6 +5,7 @@ import SwiftUI
 struct DiffSection: View, DiffGutterEditActions, DiffGutterReviewActions {
     let hunk: DiffHunk
     let rev: String?
+    var commitId: String?
     let repo: JayJayRepo?
     let actions: (any ChangeActions & DAGActions)?
     let isWorkingCopy: Bool
@@ -279,11 +280,12 @@ struct DiffSection: View, DiffGutterEditActions, DiffGutterReviewActions {
         fileDiff = nil
 
         if let cached = await diffStore.loadDiff(
-            hunk: hunk, rev: rev, repo: repo,
+            hunk: hunk, rev: rev, commitId: commitId, repo: repo,
             compareFromRev: compareFromRev,
             ignoreWhitespace: settings.ignoreWhitespace
         ) {
-            guard hunk.path == path else { return }
+            // Bail if a newer .task superseded us so we don't overwrite fresh state with a stale diff.
+            guard !Task.isCancelled, hunk.path == path else { return }
             fileDiff = cached.diff
             loadedOldContent = cached.oldContent
             loadedNewContent = cached.newContent

shell/mac/Sources/JayJay/Diff/DiffStore.swift

@@ -14,32 +14,32 @@ final class DiffStore {
     }
 
     private let cache = DiffCache()
-
-    func get(rev: String?, path: String) async -> CachedDiff? {
-        await cache.get(Self.key(rev: rev, path: path))
-    }
-
-    func set(rev: String?, path: String, value: CachedDiff) async {
-        await cache.set(Self.key(rev: rev, path: path), value: value)
-    }
+    @ObservationIgnored private var preloadTask: Task<Void, Never>?
 
     func clear() {
         Task { await cache.clear() }
     }
 
     /// Load a single file's diff. Returns cached if available, otherwise computes and caches.
+    ///
+    /// `commitId` is the immutable content hash used as the cache identity. `rev`
+    /// (the mutable selection revision) is what jj resolves to fetch content, but
+    /// keying on it would serve stale diffs after an amend/rebase reuses the id.
     func loadDiff(
         hunk: DiffHunk,
         rev: String?,
+        commitId: String? = nil,
         repo: JayJayRepo?,
         compareFromRev: String? = nil,
         ignoreWhitespace: Bool = false
     ) async -> CachedDiff? {
         guard let repo else { return nil }
         guard !hunk.isSubmodulePlaceholder else { return nil }
 
-        let cacheRev = compareFromRev != nil ? "\(compareFromRev!)→\(rev ?? "")" : rev
-        let key = Self.key(rev: cacheRev, path: hunk.path)
+        let key = Self.key(
+            commitId: commitId, rev: rev, compareFromRev: compareFromRev,
+            ignoreWhitespace: ignoreWhitespace, path: hunk.path
+        )
 
         if let cached = await cache.get(key) {
             return cached
@@ -81,28 +81,40 @@ final class DiffStore {
     func preload(
         hunks: [DiffHunk],
         rev: String?,
+        commitId: String? = nil,
         repo: JayJayRepo?,
         compareFromRev: String? = nil,
         ignoreWhitespace: Bool = false
     ) {
         guard let repo, let rev else { return }
-        preloadHunks(hunks, rev: rev, repo: repo, compareFromRev: compareFromRev, ignoreWhitespace: ignoreWhitespace)
+        // Cancel any in-flight preload so rapid commit navigation doesn't pile
+        // up detached tasks all racing the FFI.
+        preloadTask?.cancel()
+        preloadTask = preloadHunks(
+            hunks, rev: rev, commitId: commitId, repo: repo,
+            compareFromRev: compareFromRev, ignoreWhitespace: ignoreWhitespace
+        )
     }
 
     // MARK: - Private
 
+    @discardableResult
     private func preloadHunks(
         _ hunks: [DiffHunk],
         rev: String,
+        commitId: String?,
         repo: JayJayRepo,
         compareFromRev: String?,
         ignoreWhitespace: Bool
-    ) {
+    ) -> Task<Void, Never> {
         Task.detached(priority: .utility) { [cache] in
             for hunk in hunks {
+                if Task.isCancelled { return }
                 guard !hunk.isSubmodulePlaceholder else { continue }
-                let cacheRev = compareFromRev != nil ? "\(compareFromRev!)→\(rev)" : rev
-                let key = DiffStore.key(rev: cacheRev, path: hunk.path)
+                let key = DiffStore.key(
+                    commitId: commitId, rev: rev, compareFromRev: compareFromRev,
+                    ignoreWhitespace: ignoreWhitespace, path: hunk.path
+                )
                 if await cache.get(key) != nil { continue }
 
                 var old = hunk.oldContent ?? ""
@@ -198,24 +210,73 @@ final class DiffStore {
         return LoadedFileContent(oldContent: "", newContent: "", oldPreview: nil, newPreview: nil)
     }
 
-    nonisolated private static func key(rev: String?, path: String) -> String {
-        "\(rev ?? "")|\(path)"
+    /// Content-addressed cache key: immutable `commitId` (falling back to `rev`),
+    /// the compare-from side, the whitespace mode, and the path. Whitespace is
+    /// part of the key because it changes the computed diff for the same content.
+    nonisolated static func key(
+        commitId: String?,
+        rev: String?,
+        compareFromRev: String?,
+        ignoreWhitespace: Bool,
+        path: String
+    ) -> String {
+        let base = (commitId?.isEmpty == false ? commitId : rev) ?? ""
+        let identity = compareFromRev.map { "\($0)→\(base)" } ?? base
+        return "\(identity)|\(ignoreWhitespace ? "iw" : "")|\(path)"
     }
 }
 
-/// Thread-safe diff cache actor.
+/// Thread-safe LRU diff cache bounded by the total bytes of cached file content.
 actor DiffCache {
     private var entries: [String: DiffStore.CachedDiff] = [:]
+    private var order: [String] = [] // LRU recency; front = least recently used
+    private var totalBytes = 0
+    private let budgetBytes: Int
+
+    init(budgetBytes: Int = 64 * 1024 * 1024) {
+        self.budgetBytes = budgetBytes
+    }
 
     func get(_ key: String) -> DiffStore.CachedDiff? {
-        entries[key]
+        guard let value = entries[key] else { return nil }
+        touch(key)
+        return value
     }
 
     func set(_ key: String, value: DiffStore.CachedDiff) {
+        if let existing = entries[key] {
+            totalBytes -= bytes(existing)
+            order.removeAll { $0 == key }
+        }
         entries[key] = value
+        order.append(key)
+        totalBytes += bytes(value)
+        evict()
     }
 
     func clear() {
         entries.removeAll()
+        order.removeAll()
+        totalBytes = 0
+    }
+
+    private func touch(_ key: String) {
+        order.removeAll { $0 == key }
+        order.append(key)
+    }
+
+    /// Drop least-recently-used entries until under budget, always keeping the
+    /// most recent one (so a single oversized file is still cached for its view).
+    private func evict() {
+        while totalBytes > budgetBytes, order.count > 1, let oldest = order.first {
+            order.removeFirst()
+            if let removed = entries.removeValue(forKey: oldest) {
+                totalBytes -= bytes(removed)
+            }
+        }
+    }
+
+    private func bytes(_ diff: DiffStore.CachedDiff) -> Int {
+        diff.oldContent.utf8.count + diff.newContent.utf8.count
     }
 }

shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection.swift

@@ -5,6 +5,7 @@ import SwiftUI
 struct DiffEditFileSection: View, DiffGutterSelectionActions {
     let hunk: DiffHunk
     let rev: String
+    var commitId: String?
     let repo: JayJayRepo?
     let diffStore: DiffStore
     let selectedChangedLines: Set<Int>
@@ -129,7 +130,8 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions {
 
         // Reuse DiffStore for file content loading (cached if already loaded by DiffSection)
         let cached = await diffStore.loadDiff(
-            hunk: hunk, rev: rev, repo: repo, ignoreWhitespace: settings.ignoreWhitespace
+            hunk: hunk, rev: rev, commitId: commitId, repo: repo,
+            ignoreWhitespace: settings.ignoreWhitespace
         )
         let old = cached?.oldContent
         let new = cached?.newContent

shell/mac/Sources/JayJay/DiffEdit/DiffEditView.swift

@@ -32,6 +32,7 @@ struct DiffEditView: View {
                         DiffEditFileSection(
                             hunk: hunk,
                             rev: detailRevision,
+                            commitId: detail.info.commitId,
                             repo: repo,
                             diffStore: diffStore,
                             selectedChangedLines: selectedChangedLinesByPath[hunk.path] ?? [],

shell/mac/Sources/JayJay/Repo/ViewModel/Core/RepoViewModel+Selection.swift

@@ -226,10 +226,19 @@ extension RepoViewModel {
         compareDisplay = display
         selectedChangeId = to
         load {
-            try $0.interdiffSummary(fromRev: from, toRev: to)
-        } onSuccess: { viewModel, detail in
+            let detail = try $0.interdiffSummary(fromRev: from, toRev: to)
+            // Resolve the compare source to its immutable commit id so the diff
+            // cache key is content-addressed on both sides; otherwise amending a
+            // mutable `from` (a change id) would keep serving a stale interdiff.
+            let fromCommitId = (try? $0.log(revset: from))?.first?.commitId
+            return (detail, fromCommitId)
+        } onSuccess: { viewModel, result in
+            let (detail, fromCommitId) = result
             viewModel.selectedChange = detail
             viewModel.selectedChangeId = detail.info.selectionRevision
+            if let fromCommitId, !fromCommitId.isEmpty {
+                viewModel.compareFromId = fromCommitId
+            }
         } onFailure: { viewModel, error in
             viewModel.compareFromId = nil
             viewModel.compareToId = nil

shell/mac/Tests/JayJayTests/DiffCacheTests.swift

@@ -0,0 +1,79 @@
+@testable import JayJay
+import JayJayCore
+import XCTest
+
+final class DiffCacheTests: XCTestCase {
+    // MARK: - Content-addressed key
+
+    func testKeyUsesCommitIdOverRev() {
+        // Same working-copy rev, different content hash (e.g. after an edit/amend)
+        // must produce different keys so a stale diff is never served.
+        let a = DiffStore.key(commitId: "c1", rev: "@", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        let b = DiffStore.key(commitId: "c2", rev: "@", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        XCTAssertNotEqual(a, b)
+    }
+
+    func testKeyIgnoresRevWhenCommitIdPresent() {
+        // A change keeps its content hash across selection-revision spellings.
+        let a = DiffStore.key(commitId: "c1", rev: "abc", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        let b = DiffStore.key(commitId: "c1", rev: "xyz", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        XCTAssertEqual(a, b)
+    }
+
+    func testKeyFallsBackToRevWhenCommitIdMissing() {
+        let a = DiffStore.key(commitId: nil, rev: "r1", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        let b = DiffStore.key(commitId: "", rev: "r1", compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        XCTAssertEqual(a, b, "empty commit id should fall back to rev")
+    }
+
+    func testKeyDistinguishesWhitespaceMode() {
+        let on = DiffStore.key(commitId: "c1", rev: nil, compareFromRev: nil, ignoreWhitespace: true, path: "f")
+        let off = DiffStore.key(commitId: "c1", rev: nil, compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        XCTAssertNotEqual(on, off)
+    }
+
+    func testKeyDistinguishesCompareSide() {
+        let plain = DiffStore.key(commitId: "c1", rev: nil, compareFromRev: nil, ignoreWhitespace: false, path: "f")
+        let compare = DiffStore.key(commitId: "c1", rev: nil, compareFromRev: "c0", ignoreWhitespace: false, path: "f")
+        XCTAssertNotEqual(plain, compare)
+    }
+
+    // MARK: - LRU eviction
+
+    func testEvictsLeastRecentlyUsedPastBudget() async {
+        let cache = DiffCache(budgetBytes: 100)
+        await cache.set("A", value: entry(bytes: 60))
+        await cache.set("B", value: entry(bytes: 30)) // total 90, both fit
+
+        let a = await cache.get("A") // touch A → B is now least recently used
+        XCTAssertNotNil(a)
+
+        await cache.set("C", value: entry(bytes: 30)) // total 120 > 100 → evict LRU (B)
+
+        let evicted = await cache.get("B")
+        let keptA = await cache.get("A")
+        let keptC = await cache.get("C")
+        XCTAssertNil(evicted, "B was least recently used and should be evicted")
+        XCTAssertNotNil(keptA)
+        XCTAssertNotNil(keptC)
+    }
+
+    func testKeepsSingleEntryLargerThanBudget() async {
+        let cache = DiffCache(budgetBytes: 100)
+        await cache.set("big", value: entry(bytes: 500))
+        let stored = await cache.get("big")
+        XCTAssertNotNil(stored, "the only entry is kept even when it exceeds the budget")
+    }
+
+    // MARK: - Fixtures
+
+    private func entry(bytes: Int) -> DiffStore.CachedDiff {
+        DiffStore.CachedDiff(
+            diff: FileDiff(path: "f", language: "", lines: [], whitespaceOnlyHidden: false),
+            oldContent: "",
+            newContent: String(repeating: "x", count: bytes),
+            oldPreview: nil,
+            newPreview: nil
+        )
+    }
+}