diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 36dad82..23c81b1 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -391,8 +391,17 @@ export function App({ wrapLines, }); - await onReloadSession(nextInput, { resetApp: false }); + await onReloadSession(nextInput, { + resetApp: false, + sourcePath: + bootstrap.input.kind === "git" || + bootstrap.input.kind === "show" || + bootstrap.input.kind === "stash-show" + ? bootstrap.changeset.sourceLabel + : undefined, + }); }, [ + bootstrap.changeset.sourceLabel, bootstrap.input, canRefreshCurrentInput, layoutMode, diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index c8c5667..c0230a1 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -2,9 +2,53 @@ import { useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; import type { DiffFile } from "../../core/types"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./pierre"; +/** Maximum cached highlight results. Prevents unbounded growth during long watch sessions. */ +const MAX_CACHE_ENTRIES = 150; + const SHARED_HIGHLIGHTED_DIFF_CACHE = new Map(); const SHARED_HIGHLIGHT_PROMISES = new Map>(); +/** Evict the oldest entries when the cache exceeds MAX_CACHE_ENTRIES. + * Map iteration order is insertion order, so the first keys are the oldest. */ +function enforceCacheLimit() { + while (SHARED_HIGHLIGHTED_DIFF_CACHE.size > MAX_CACHE_ENTRIES) { + const oldest = SHARED_HIGHLIGHTED_DIFF_CACHE.keys().next().value; + if (oldest !== undefined) { + SHARED_HIGHLIGHTED_DIFF_CACHE.delete(oldest); + } + } +} + +/** Content fingerprint from the diff patch. Changes whenever the underlying diff + * changes, allowing per-file cache invalidation without a global flush. */ +function patchFingerprint(file: DiffFile) { + const { patch } = file; + const mid = Math.floor(patch.length / 2); + return `${patch.length}:${patch.slice(0, 64)}:${patch.slice(mid, mid + 64)}:${patch.slice(-64)}`; +} + +/** Cache key that includes a content fingerprint so stale entries are never served + * after reload. Unchanged files keep their cache hit across reloads. */ +function buildCacheKey(appearance: string, file: DiffFile) { + return `${appearance}:${file.id}:${patchFingerprint(file)}`; +} + +/** Only commit a highlight result if the promise is still the active one for that key. + * Prevents a superseded or late-resolving promise from overwriting a newer entry. */ +function commitHighlightResult( + cacheKey: string, + promise: Promise, + result: HighlightedDiffCode, +) { + if (SHARED_HIGHLIGHT_PROMISES.get(cacheKey) !== promise) { + return false; + } + SHARED_HIGHLIGHT_PROMISES.delete(cacheKey); + SHARED_HIGHLIGHTED_DIFF_CACHE.set(cacheKey, result); + enforceCacheLimit(); + return true; +} + /** Resolve highlighted diff content with shared caching and background prefetch support. */ export function useHighlightedDiff({ file, @@ -19,7 +63,7 @@ export function useHighlightedDiff({ }) { const [highlighted, setHighlighted] = useState(null); const [highlightedCacheKey, setHighlightedCacheKey] = useState(null); - const appearanceCacheKey = file ? `${appearance}:${file.id}` : null; + const appearanceCacheKey = file ? buildCacheKey(appearance, file) : null; // Selected files load immediately; background prefetch can opt neighboring files in later. const pendingHighlight = useMemo(() => { @@ -67,30 +111,35 @@ export function useHighlightedDiff({ let cancelled = false; setHighlighted(null); - pendingHighlight + // Capture the key and promise reference this effect was started for so the + // resolution callback only writes if it is still the active request. + const effectCacheKey = appearanceCacheKey; + const effectPromise = pendingHighlight; + + effectPromise ?.then((nextHighlighted) => { if (cancelled) { return; } - SHARED_HIGHLIGHT_PROMISES.delete(appearanceCacheKey); - SHARED_HIGHLIGHTED_DIFF_CACHE.set(appearanceCacheKey, nextHighlighted); - setHighlighted(nextHighlighted); - setHighlightedCacheKey(appearanceCacheKey); + if (commitHighlightResult(effectCacheKey, effectPromise, nextHighlighted)) { + setHighlighted(nextHighlighted); + setHighlightedCacheKey(effectCacheKey); + } }) .catch(() => { if (cancelled) { return; } - SHARED_HIGHLIGHT_PROMISES.delete(appearanceCacheKey); const fallback = { deletionLines: [], additionLines: [], } satisfies HighlightedDiffCode; - SHARED_HIGHLIGHTED_DIFF_CACHE.set(appearanceCacheKey, fallback); - setHighlighted(fallback); - setHighlightedCacheKey(appearanceCacheKey); + if (commitHighlightResult(effectCacheKey, effectPromise, fallback)) { + setHighlighted(fallback); + setHighlightedCacheKey(effectCacheKey); + } }); return () => { diff --git a/test/reload-bug.test.tsx b/test/reload-bug.test.tsx new file mode 100644 index 0000000..b7eda36 --- /dev/null +++ b/test/reload-bug.test.tsx @@ -0,0 +1,137 @@ +import { execSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, test } from "bun:test"; +import { testRender } from "@opentui/react/test-utils"; +import { act } from "react"; + +const { loadAppBootstrap } = await import("../src/core/loaders"); +const { AppHost } = await import("../src/ui/AppHost"); + +async function flush(setup: Awaited>) { + await act(async () => { + await setup.renderOnce(); + await Bun.sleep(0); + await setup.renderOnce(); + }); +} + +/** Settle renders long enough for the async syntax-highlight cache to populate. + * Without this, the plain-text fallback path masks the stale-cache bug. */ +async function settleHighlights(setup: Awaited>) { + for (let i = 0; i < 15; i++) { + await flush(setup); + await Bun.sleep(50); + } +} + +describe("reload stale highlight cache", () => { + test("r key picks up new file content for file-pair diffs", async () => { + const dir = mkdtempSync(join(tmpdir(), "hunk-reload-file-")); + const left = join(dir, "before.ts"); + const right = join(dir, "after.ts"); + + writeFileSync(left, "export const answer = 41;\n"); + writeFileSync(right, "export const answer = 42;\nexport const first = true;\n"); + + const bootstrap = await loadAppBootstrap({ + kind: "diff", + left, + right, + options: { mode: "stack" }, + }); + + const setup = await testRender(, { + width: 220, + height: 20, + }); + + try { + await settleHighlights(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("first"); + + // Modify the right file while hunk is open + writeFileSync(right, "export const answer = 42;\nexport const second = true;\n"); + + await act(async () => { + await setup.mockInput.typeText("r"); + }); + + let refreshed = false; + for (let attempt = 0; attempt < 30; attempt++) { + await flush(setup); + frame = setup.captureCharFrame(); + if (frame.includes("second") && !frame.includes("first")) { + refreshed = true; + break; + } + await Bun.sleep(50); + } + + expect(refreshed).toBe(true); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + rmSync(dir, { force: true, recursive: true }); + } + }); + + test("r key picks up new file content for git working tree diffs", async () => { + const dir = mkdtempSync(join(tmpdir(), "hunk-reload-git-")); + const file = join(dir, "test.txt"); + + execSync("git init && git config user.email test@test && git config user.name test", { + cwd: dir, + stdio: "ignore", + }); + writeFileSync(file, "original line\n"); + execSync("git add . && git commit -m init", { cwd: dir, stdio: "ignore" }); + + writeFileSync(file, "original line\nfirst change\n"); + + const bootstrap = await loadAppBootstrap( + { kind: "git", staged: false, options: { mode: "stack", excludeUntracked: true } }, + { cwd: dir }, + ); + + const setup = await testRender(, { + width: 120, + height: 20, + }); + + try { + await settleHighlights(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("first change"); + + writeFileSync(file, "original line\nsecond change\n"); + + await act(async () => { + await setup.mockInput.typeText("r"); + }); + + let refreshed = false; + for (let attempt = 0; attempt < 30; attempt++) { + await flush(setup); + frame = setup.captureCharFrame(); + if (frame.includes("second change") && !frame.includes("first change")) { + refreshed = true; + break; + } + await Bun.sleep(50); + } + + expect(refreshed).toBe(true); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + rmSync(dir, { force: true, recursive: true }); + } + }); +});