perf(picker): refresh stale Log preview cache in background after disk hit

src/commands/picker/items.rs

@@ -233,7 +233,10 @@ impl WorktreeSkimItem {
         format!("○ {verb} {label}. Press {key} again to refresh.\n")
     }
 
-    /// Compute preview and apply pager for diff modes.
+    /// Compute preview and apply pager for diff modes. Returns the
+    /// display-ready string and (for Log) whether the disk cache was a
+    /// hit — the orchestrator uses the flag to schedule a background
+    /// refresh.
     ///
     /// Both the inline cache-miss path and background pre-computation use this so
     /// that the cache always stores display-ready content (no pager subprocess
@@ -244,34 +247,33 @@ impl WorktreeSkimItem {
         mode: PreviewMode,
         width: usize,
         height: usize,
-    ) -> String {
-        let content = Self::compute_preview(repo, item, mode, width, height);
+    ) -> (String, bool) {
         match mode {
-            PreviewMode::WorkingTree | PreviewMode::BranchDiff | PreviewMode::UpstreamDiff => {
-                if let Some(pager_cmd) = diff_pager() {
-                    pipe_through_pager(&content, pager_cmd, width)
-                } else {
-                    content
-                }
-            }
-            _ => content,
+            PreviewMode::WorkingTree => (
+                Self::page_diff(Self::compute_working_tree_preview(repo, item, width), width),
+                false,
+            ),
+            PreviewMode::Log => Self::compute_log_preview(repo, item, width, height),
+            PreviewMode::BranchDiff => (
+                Self::page_diff(Self::compute_branch_diff_preview(repo, item, width), width),
+                false,
+            ),
+            PreviewMode::UpstreamDiff => (
+                Self::page_diff(
+                    Self::compute_upstream_diff_preview(repo, item, width),
+                    width,
+                ),
+                false,
+            ),
+            PreviewMode::Summary => (Self::loading_placeholder(PreviewMode::Summary), false),
         }
     }
 
-    /// Compute raw preview for any mode.
-    pub(super) fn compute_preview(
-        repo: &Repository,
-        item: &ListItem,
-        mode: PreviewMode,
-        width: usize,
-        height: usize,
-    ) -> String {
-        match mode {
-            PreviewMode::WorkingTree => Self::compute_working_tree_preview(repo, item, width),
-            PreviewMode::Log => Self::compute_log_preview(repo, item, width, height),
-            PreviewMode::BranchDiff => Self::compute_branch_diff_preview(repo, item, width),
-            PreviewMode::UpstreamDiff => Self::compute_upstream_diff_preview(repo, item, width),
-            PreviewMode::Summary => Self::loading_placeholder(PreviewMode::Summary),
+    fn page_diff(content: String, width: usize) -> String {
+        if let Some(pager_cmd) = diff_pager() {
+            pipe_through_pager(&content, pager_cmd, width)
+        } else {
+            content
         }
     }
 
@@ -442,12 +444,40 @@ impl WorktreeSkimItem {
     /// key out of `main`'s SHA — a `git fetch` advancing `origin/main`
     /// doesn't invalidate any entry — while preserving correctness as
     /// `main` and wall-clock advance.
+    /// Render the Log preview and report whether the disk cache was hit.
+    /// The orchestrator uses the flag to schedule a background refresh —
+    /// see [`Self::refresh_log_preview`] and the `LogCacheEntry`
+    /// docstring for why decorations baked into `raw_log` need
+    /// refreshing even though the cache key is correct.
     pub(super) fn compute_log_preview(
         repo: &Repository,
         item: &ListItem,
         width: usize,
         height: usize,
+    ) -> (String, bool) {
+        Self::compute_log_preview_inner(repo, item, width, height, false)
+    }
+
+    /// Force-recompute the Log preview, bypassing the disk-cache read but
+    /// writing through. Returns the freshly rendered string. Called by the
+    /// orchestrator's refresh worker after a cache hit so the next visit
+    /// sees decorations that match current ref topology.
+    pub(super) fn refresh_log_preview(
+        repo: &Repository,
+        item: &ListItem,
+        width: usize,
+        height: usize,
     ) -> String {
+        Self::compute_log_preview_inner(repo, item, width, height, true).0
+    }
+
+    fn compute_log_preview_inner(
+        repo: &Repository,
+        item: &ListItem,
+        width: usize,
+        height: usize,
+        force_recompute: bool,
+    ) -> (String, bool) {
         // Minimum preview width to show timestamps (adds ~7 chars: space + 4-char time + space)
         // Note: preview is typically 50% of terminal width, so 50 = 100-col terminal
         const TIMESTAMP_WIDTH_THRESHOLD: usize = 50;
@@ -461,7 +491,10 @@ impl WorktreeSkimItem {
         let branch = item.branch_name();
         let reset = Reset;
         let Some(default_branch) = repo.default_branch() else {
-            return cformat!("{INFO_SYMBOL}{reset} <bold>{branch}</>{reset} has no commits\n");
+            return (
+                cformat!("{INFO_SYMBOL}{reset} <bold>{branch}</>{reset} has no commits\n"),
+                false,
+            );
         };
 
         // merge-base / rev-list run on every call — they're how the
@@ -473,7 +506,10 @@ impl WorktreeSkimItem {
         // the preview is supplementary, users can still select worktrees
         // even if a probe fails.
         let Ok(merge_base_output) = repo.run_command(&["merge-base", &default_branch, head]) else {
-            return cformat!("{INFO_SYMBOL}{reset} <bold>{branch}</>{reset} has no commits\n");
+            return (
+                cformat!("{INFO_SYMBOL}{reset} <bold>{branch}</>{reset} has no commits\n"),
+                false,
+            );
         };
         let merge_base = merge_base_output.trim();
         let is_default_branch = branch == default_branch;
@@ -500,32 +536,38 @@ impl WorktreeSkimItem {
         // Cacheable: the raw `git log --graph` output plus per-commit
         // stats. Both are pure functions of (head, width, height); on a
         // disk-cache hit we skip the `git log` and `git diff-tree` calls
-        // entirely. On miss we compute and write through.
-        let entry = match preview_cache::read_log(repo, head, width, height) {
-            Some(cached) => cached,
-            None => {
-                let Some(fresh) =
-                    Self::compute_log_raw_and_stats(repo, head, log_limit, show_timestamps)
-                else {
-                    // Match prior behavior: empty output on `git log` failure.
-                    return String::new();
-                };
-                preview_cache::write_log(repo, head, width, height, &fresh);
-                fresh
-            }
+        // entirely. On miss we compute and write through. `force_recompute`
+        // bypasses the read (the refresh path) but always writes.
+        let cached = if force_recompute {
+            None
+        } else {
+            preview_cache::read_log(repo, head, width, height)
         };
+        let was_disk_hit = cached.is_some();
+        // On `git log` failure (effectively unreachable here — merge-base
+        // already validated `head` upstream), fall back to a default
+        // entry. `process_log_with_dimming` + `format_log_output` handle
+        // the empty `raw_log` gracefully and produce empty output, the
+        // same effective behavior as the prior explicit early return.
+        let entry = cached.unwrap_or_else(|| {
+            let fresh = Self::compute_log_raw_and_stats(repo, head, log_limit, show_timestamps)
+                .unwrap_or_default();
+            preview_cache::write_log(repo, head, width, height, &fresh);
+            fresh
+        });
 
         let (processed, _hashes) =
             process_log_with_dimming(&entry.raw_log, unique_commits.as_ref());
-        if show_timestamps {
+        let rendered = if show_timestamps {
             // `format_log_output` reads `epoch_now()` so relative-time
             // strings ("5m" / "2h" / "3d") track wall-clock on every call,
             // even when serving from cache.
             format_log_output(&processed, &entry.stats)
         } else {
             // Strip hash markers (SOH...NUL) since we're not using format_log_output
             strip_hash_markers(&processed)
-        }
+        };
+        (rendered, was_disk_hit)
     }
 
     /// Run `git log --graph` and (when timestamps are shown) `batch_fetch_stats`,
@@ -784,7 +826,7 @@ mod tests {
         let item = item_at(&repo, "feature");
 
         assert!(super::preview_cache::read_log(&repo, item.head(), 80, 24).is_none());
-        let _ = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24);
+        let _ = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24).0;
         let entry = super::preview_cache::read_log(&repo, item.head(), 80, 24)
             .expect("cache populated after first compute");
         assert!(
@@ -829,7 +871,7 @@ mod tests {
         // which removes that escape. The dim SGR `\x1b[2m` is unsuitable
         // because `format_log_output` already wraps every relative-time
         // column in dim, so it appears even in bright lines.
-        let before = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24);
+        let before = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24).0;
         let before_subject_line = before
             .lines()
             .find(|l| l.contains("feature commit"))
@@ -847,7 +889,7 @@ mod tests {
             .unwrap();
         repo.run_command(&["checkout", "feature"]).unwrap();
 
-        let after = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24);
+        let after = WorktreeSkimItem::compute_log_preview(&repo, &item, 80, 24).0;
         let after_subject_line = after
             .lines()
             .find(|l| l.contains("feature commit"))

src/commands/picker/preview_cache.rs

@@ -49,7 +49,20 @@ const KIND: &str = "picker-preview";
 /// `batch_fetch_stats`. The render path re-runs `process_log_with_dimming`
 /// against fresh `unique_commits` and `format_log_output` against
 /// `epoch_now()`, so output stays correct as `main` and wall-clock advance.
-#[derive(Serialize, Deserialize)]
+///
+/// `raw_log` also embeds `git log --format=%C(auto)%d` ref decorations
+/// (e.g. `HEAD -> feature, main`), which are *not* SHA-deterministic —
+/// another ref starting or stopping pointing at the same commit (most
+/// commonly: a squash merge landing `main` at the cached SHA) would
+/// leave the decoration text stale even though the cache key is still
+/// valid. The orchestrator mitigates this by enqueuing a background
+/// refresh task on the picker's rayon pool whenever a Log preview hit
+/// the disk cache; the refresh re-runs `compute_log_raw_and_stats`,
+/// overwrites this entry, and updates the in-memory `PreviewCache`.
+/// The cached entry served on the *current* render is still potentially
+/// stale — refresh is async — but the next visit to the same row reads
+/// fresh content. See `commands::picker::preview_orchestrator::spawn_preview`.
+#[derive(Serialize, Deserialize, Default)]
 pub(super) struct LogCacheEntry {
     pub raw_log: String,
     /// Empty when `width < TIMESTAMP_WIDTH_THRESHOLD` (the no-timestamp

src/commands/picker/preview_orchestrator.rs

@@ -62,6 +62,12 @@ impl PreviewOrchestrator {
     /// happens outside any DashMap lock so skim's UI thread (which calls
     /// `preview()` synchronously and reads via `DashMap::get`) is never
     /// blocked on a shard write held across git/pager subprocesses.
+    ///
+    /// Log mode that hits the disk cache also enqueues a refresh task
+    /// (via `pool.spawn_fifo`, so it lands behind in-flight foreground
+    /// precompute) to recompute the embedded ref decorations before the
+    /// next visit. See the `LogCacheEntry` docstring for why the disk
+    /// cache itself is SHA-keyed but decoration text drifts.
     pub(super) fn spawn_preview(
         &self,
         item: Arc<ListItem>,
@@ -71,13 +77,33 @@ impl PreviewOrchestrator {
         let cache = Arc::clone(&self.cache);
         let (w, h) = dims;
         let repo = self.repo.clone();
+        let pool = Arc::clone(&self.pool);
+        let pending = Arc::clone(&self.pending);
         self.spawn_task(move || {
             let cache_key = (item.branch_name().to_string(), mode);
             if cache.contains_key(&cache_key) {
                 return;
             }
-            let value = WorktreeSkimItem::compute_and_page_preview(&repo, &item, mode, w, h);
+            let (value, log_disk_hit) =
+                WorktreeSkimItem::compute_and_page_preview(&repo, &item, mode, w, h);
             cache.insert(cache_key, value);
+            if log_disk_hit {
+                pending.fetch_add(1, Ordering::SeqCst);
+                let guard = PendingGuard(Arc::clone(&pending));
+                let item = Arc::clone(&item);
+                let cache = Arc::clone(&cache);
+                let repo = repo.clone();
+                pool.spawn_fifo(move || {
+                    let _g = guard;
+                    let rendered = WorktreeSkimItem::refresh_log_preview(&repo, &item, w, h);
+                    // Skip empty results so a transient `git log` failure
+                    // doesn't poison the in-memory cache with "" and wipe
+                    // out the value the producer just inserted.
+                    if !rendered.is_empty() {
+                        cache.insert((item.branch_name().to_string(), PreviewMode::Log), rendered);
+                    }
+                });
+            }
         });
     }
 
@@ -293,6 +319,79 @@ mod tests {
         );
     }
 
+    /// Disk-cache hit on a Log preview enqueues a background refresh that
+    /// overwrites both the disk file and the in-memory DashMap. Seed the
+    /// disk cache with a stale `LogCacheEntry` containing a marker —
+    /// after `spawn_preview` + `wait_for_idle`, neither cache should
+    /// hold the marker, because the refresh thread re-ran
+    /// `compute_log_raw_and_stats` and wrote real git-log output.
+    ///
+    /// `wait_for_idle` covers the refresh thread's task because the
+    /// producer increments `pending` before sending and the refresh
+    /// thread decrements via `PendingGuard` after running.
+    #[test]
+    fn log_disk_hit_triggers_background_refresh() {
+        let (t, item) = dirty_worktree_item();
+        let repo = Repository::at(t.path()).unwrap();
+
+        let stale = super::super::preview_cache::LogCacheEntry {
+            raw_log: "STALE_MARKER\n".to_string(),
+            stats: std::collections::HashMap::new(),
+        };
+        super::super::preview_cache::write_log(&repo, item.head(), 80, 24, &stale);
+
+        let orch = orch_for(&t);
+        orch.spawn_preview(Arc::clone(&item), PreviewMode::Log, (80, 24));
+        orch.wait_for_idle();
+
+        let disk = super::super::preview_cache::read_log(&repo, item.head(), 80, 24)
+            .expect("disk cache present after refresh");
+        assert!(
+            !disk.raw_log.contains("STALE_MARKER"),
+            "refresh should overwrite stale disk entry, got raw_log: {:?}",
+            disk.raw_log
+        );
+
+        let in_memory = orch
+            .cache
+            .get(&("main".to_string(), PreviewMode::Log))
+            .expect("in-memory entry present")
+            .clone();
+        assert!(
+            !in_memory.contains("STALE_MARKER"),
+            "refresh should overwrite stale in-memory entry, got: {in_memory:?}"
+        );
+    }
+
+    /// Non-Log modes have content-addressed cache keys (BranchDiff is
+    /// `(base_sha, branch_sha, w)`, UpstreamDiff similar) and no
+    /// decoration drift, so a disk-cache hit on those modes must NOT
+    /// enqueue a Log refresh. Seed the disk Log cache with stale content
+    /// and spawn a BranchDiff preview — the disk Log cache must remain
+    /// stale because the refresh thread never received a task.
+    #[test]
+    fn non_log_modes_do_not_trigger_log_refresh() {
+        let (t, item) = dirty_worktree_item();
+        let repo = Repository::at(t.path()).unwrap();
+
+        let stale = super::super::preview_cache::LogCacheEntry {
+            raw_log: "STALE_MARKER\n".to_string(),
+            stats: std::collections::HashMap::new(),
+        };
+        super::super::preview_cache::write_log(&repo, item.head(), 80, 24, &stale);
+
+        let orch = orch_for(&t);
+        orch.spawn_preview(Arc::clone(&item), PreviewMode::BranchDiff, (80, 24));
+        orch.wait_for_idle();
+
+        let disk = super::super::preview_cache::read_log(&repo, item.head(), 80, 24)
+            .expect("disk Log cache untouched");
+        assert_eq!(
+            disk.raw_log, "STALE_MARKER\n",
+            "non-Log spawn must not trigger Log refresh"
+        );
+    }
+
     #[test]
     fn dump_cache_json_format() {
         let t = TestRepo::new();