Skip to content
Open
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
11 changes: 10 additions & 1 deletion src/ui/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
69 changes: 59 additions & 10 deletions src/ui/diff/useHighlightedDiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, HighlightedDiffCode>();
const SHARED_HIGHLIGHT_PROMISES = new Map<string, Promise<HighlightedDiffCode>>();

/** 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<HighlightedDiffCode>,
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,
Expand All @@ -19,7 +63,7 @@ export function useHighlightedDiff({
}) {
const [highlighted, setHighlighted] = useState<HighlightedDiffCode | null>(null);
const [highlightedCacheKey, setHighlightedCacheKey] = useState<string | null>(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(() => {
Expand Down Expand Up @@ -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 () => {
Expand Down
137 changes: 137 additions & 0 deletions test/reload-bug.test.tsx
Original file line number Diff line number Diff line change
@@ -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<ReturnType<typeof testRender>>) {
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<ReturnType<typeof testRender>>) {
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(<AppHost bootstrap={bootstrap} />, {
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(<AppHost bootstrap={bootstrap} />, {
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 });
}
});
});