From 2af777224aa1f83098436cd6f51b6096ec04cfd7 Mon Sep 17 00:00:00 2001 From: Edu Wass Date: Tue, 31 Mar 2026 03:51:54 +0200 Subject: [PATCH 1/2] fix(reload): stale syntax highlights after r key / watch reload The highlight cache keyed by appearance + file.id served stale HAST nodes after reload because the file ID is path-based and doesn't change when content changes. Switch to a content-addressed cache key using a patch fingerprint, add promise race guards, and cap the cache at 150 entries. --- src/ui/App.tsx | 6 +- src/ui/diff/useHighlightedDiff.ts | 70 ++++++++++++--- test/reload-bug.test.tsx | 137 ++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 test/reload-bug.test.tsx diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 36dad82..95126dd 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -391,8 +391,12 @@ export function App({ wrapLines, }); - await onReloadSession(nextInput, { resetApp: false }); + await onReloadSession(nextInput, { + resetApp: false, + sourcePath: bootstrap.changeset.sourceLabel, + }); }, [ + bootstrap.changeset.sourceLabel, bootstrap.input, canRefreshCurrentInput, layoutMode, diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index c8c5667..5c3770f 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -2,9 +2,51 @@ 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) { + return `${file.patch.length}:${file.patch.slice(0, 128)}`; +} + +/** 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 +61,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 +109,38 @@ 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 ( + effectPromise && + 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 (effectPromise && 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 }); + } + }); +}); From 67a32233618aee058d23427ec864e13fa905cabb Mon Sep 17 00:00:00 2001 From: Edu Wass Date: Tue, 31 Mar 2026 04:09:55 +0200 Subject: [PATCH 2/2] fix: address review feedback - Guard sourcePath on git-backed input kinds only (diff/difftool use display strings like "file compare" as sourceLabel, not paths) - Strengthen patchFingerprint to sample start, middle, and end of the patch to avoid collisions on edits deep in large files - Remove redundant effectPromise && guards inside .then()/.catch() --- src/ui/App.tsx | 7 ++++++- src/ui/diff/useHighlightedDiff.ts | 11 +++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 95126dd..23c81b1 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -393,7 +393,12 @@ export function App({ await onReloadSession(nextInput, { resetApp: false, - sourcePath: bootstrap.changeset.sourceLabel, + sourcePath: + bootstrap.input.kind === "git" || + bootstrap.input.kind === "show" || + bootstrap.input.kind === "stash-show" + ? bootstrap.changeset.sourceLabel + : undefined, }); }, [ bootstrap.changeset.sourceLabel, diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index 5c3770f..c0230a1 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -22,7 +22,9 @@ function enforceCacheLimit() { /** 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) { - return `${file.patch.length}:${file.patch.slice(0, 128)}`; + 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 @@ -120,10 +122,7 @@ export function useHighlightedDiff({ return; } - if ( - effectPromise && - commitHighlightResult(effectCacheKey, effectPromise, nextHighlighted) - ) { + if (commitHighlightResult(effectCacheKey, effectPromise, nextHighlighted)) { setHighlighted(nextHighlighted); setHighlightedCacheKey(effectCacheKey); } @@ -137,7 +136,7 @@ export function useHighlightedDiff({ deletionLines: [], additionLines: [], } satisfies HighlightedDiffCode; - if (effectPromise && commitHighlightResult(effectCacheKey, effectPromise, fallback)) { + if (commitHighlightResult(effectCacheKey, effectPromise, fallback)) { setHighlighted(fallback); setHighlightedCacheKey(effectCacheKey); }