From ffc4d69b6b8099e1cfee402514b0b98c230123ee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 17:30:15 +0000 Subject: [PATCH 1/5] perf(diff): evict highlight cache by recency, not highlight age The shared highlight cache evicted in insertion order and reads never re-inserted, so entries expired by age-since-highlight instead of age-since-use. Once the review warmed more than 40 files, the file on screen could be evicted by prefetch for files ahead of it and then have to be re-highlighted on the way back. Move the store into its own module and refresh recency on read. Viewport prefetch re-reads its whole halo on every scroll, so the files around the viewport now stay resident and eviction falls on files the review has scrolled away from. Render reads peek instead, keeping the render path side-effect free while the commit-phase effect does the touching. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P2q34GvqwwMLbnBV7hjBB4 --- .changeset/lru-highlight-cache.md | 5 ++ src/ui/diff/highlightedDiffCache.test.ts | 66 ++++++++++++++++++++++ src/ui/diff/highlightedDiffCache.ts | 71 ++++++++++++++++++++++++ src/ui/diff/useHighlightedDiff.ts | 31 +++-------- 4 files changed, 150 insertions(+), 23 deletions(-) create mode 100644 .changeset/lru-highlight-cache.md create mode 100644 src/ui/diff/highlightedDiffCache.test.ts create mode 100644 src/ui/diff/highlightedDiffCache.ts diff --git a/.changeset/lru-highlight-cache.md b/.changeset/lru-highlight-cache.md new file mode 100644 index 000000000..b097eb8cb --- /dev/null +++ b/.changeset/lru-highlight-cache.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Keep syntax highlighting cached for the files you are actually reviewing, so scrolling back to a recent file no longer re-highlights it. diff --git a/src/ui/diff/highlightedDiffCache.test.ts b/src/ui/diff/highlightedDiffCache.test.ts new file mode 100644 index 000000000..f7198ce1e --- /dev/null +++ b/src/ui/diff/highlightedDiffCache.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test"; +import type { HighlightedDiffCode } from "./diffRows"; +import { createHighlightedDiffCache } from "./highlightedDiffCache"; + +/** Build a distinguishable cache value; identity is all these tests compare. */ +function createTestHighlightedDiffCode(): HighlightedDiffCode { + return { deletionLines: [], additionLines: [] }; +} + +describe("highlighted diff cache", () => { + test("evicts the least recently used entry rather than the oldest highlight", () => { + const cache = createHighlightedDiffCache(2); + const onScreen = createTestHighlightedDiffCode(); + const scrolledPast = createTestHighlightedDiffCode(); + const prefetched = createTestHighlightedDiffCode(); + + cache.set("on-screen", onScreen); + cache.set("scrolled-past", scrolledPast); + + // The viewport prefetch re-reads the file the user is looking at before warming the next one. + expect(cache.get("on-screen")).toBe(onScreen); + cache.set("prefetched", prefetched); + + expect(cache.peek("on-screen")).toBe(onScreen); + expect(cache.peek("prefetched")).toBe(prefetched); + expect(cache.peek("scrolled-past")).toBeUndefined(); + }); + + test("peeking does not protect an entry from eviction", () => { + const cache = createHighlightedDiffCache(1); + const first = createTestHighlightedDiffCode(); + const second = createTestHighlightedDiffCode(); + + cache.set("first", first); + expect(cache.peek("first")).toBe(first); + + cache.set("second", second); + expect(cache.peek("first")).toBeUndefined(); + expect(cache.peek("second")).toBe(second); + }); + + test("re-storing a key refreshes recency without growing past the budget", () => { + const cache = createHighlightedDiffCache(2); + const replaced = createTestHighlightedDiffCode(); + const kept = createTestHighlightedDiffCode(); + const added = createTestHighlightedDiffCode(); + + cache.set("reloaded", createTestHighlightedDiffCode()); + cache.set("kept", kept); + cache.set("reloaded", replaced); + cache.set("added", added); + + expect(cache.peek("reloaded")).toBe(replaced); + expect(cache.peek("added")).toBe(added); + expect(cache.peek("kept")).toBeUndefined(); + }); + + test("keeps at least one entry when given a degenerate budget", () => { + const cache = createHighlightedDiffCache(0); + const only = createTestHighlightedDiffCode(); + + cache.set("only", only); + + expect(cache.peek("only")).toBe(only); + }); +}); diff --git a/src/ui/diff/highlightedDiffCache.ts b/src/ui/diff/highlightedDiffCache.ts new file mode 100644 index 000000000..5ee4db784 --- /dev/null +++ b/src/ui/diff/highlightedDiffCache.ts @@ -0,0 +1,71 @@ +import type { HighlightedDiffCode } from "./diffRows"; + +/** + * Maximum cached highlight results. + * + * Highlighted HAST nodes and their flattened render spans are expensive enough that a whole-review + * cache can dominate memory while navigating large changesets. Keep a viewport-local working set + * instead of retaining every file the user has visited in the current review. + */ +const MAX_HIGHLIGHTED_DIFF_CACHE_ENTRIES = 40; + +export interface HighlightedDiffCache { + /** Read one result and mark it most recently used. */ + get: (key: string) => HighlightedDiffCode | undefined; + /** Read one result without changing recency, for render paths that must stay side-effect free. */ + peek: (key: string) => HighlightedDiffCode | undefined; + /** Store one result as most recently used, evicting the least recently used entries over budget. */ + set: (key: string, value: HighlightedDiffCode) => void; +} + +/** + * Holds highlight results under a bounded least-recently-used budget. + * + * Reads refresh recency, so eviction drops the files the review has stopped touching rather than + * the ones highlighted longest ago. Viewport prefetch re-reads its whole halo on every scroll, so + * the files on screen stay resident while files ahead of them are warmed. A halo wider than the + * budget still thrashes; that is a prefetch sizing question, not an eviction-order one. + */ +export function createHighlightedDiffCache( + maxEntries = MAX_HIGHLIGHTED_DIFF_CACHE_ENTRIES, +): HighlightedDiffCache { + const entries = new Map(); + const budget = Math.max(1, Math.floor(maxEntries)); + + /** Move one key to the most-recently-used end of Map iteration order. */ + const touch = (key: string, value: HighlightedDiffCode) => { + entries.delete(key); + entries.set(key, value); + }; + + return { + get(key) { + const entry = entries.get(key); + if (entry === undefined) { + return undefined; + } + + touch(key, entry); + return entry; + }, + + peek(key) { + return entries.get(key); + }, + + set(key, value) { + touch(key, value); + + // Map iteration order is insertion order and every read re-inserts, so the first keys are + // the least recently used. + while (entries.size > budget) { + const leastRecentlyUsed = entries.keys().next().value; + if (leastRecentlyUsed === undefined) { + return; + } + + entries.delete(leastRecentlyUsed); + } + }, + }; +} diff --git a/src/ui/diff/useHighlightedDiff.ts b/src/ui/diff/useHighlightedDiff.ts index 637c025c4..1a9f66dc0 100644 --- a/src/ui/diff/useHighlightedDiff.ts +++ b/src/ui/diff/useHighlightedDiff.ts @@ -2,33 +2,14 @@ import { useLayoutEffect, useState } from "react"; import type { DiffFile } from "../../core/types"; import type { AppTheme } from "../themes"; import { loadHighlightedDiff, type HighlightedDiffCode } from "./diffRows"; +import { createHighlightedDiffCache } from "./highlightedDiffCache"; import { syntaxHighlightThemeName } from "./syntaxHighlightTheme"; -/** - * Maximum cached highlight results. - * - * Highlighted HAST nodes and their flattened render spans are expensive enough that a whole-review - * cache can dominate memory while navigating large changesets. Keep a viewport-local working set - * instead of retaining every file the user has visited in the current review. - */ -const MAX_CACHE_ENTRIES = 40; - -const SHARED_HIGHLIGHTED_DIFF_CACHE = new Map(); +const SHARED_HIGHLIGHTED_DIFF_CACHE = createHighlightedDiffCache(); const SHARED_HIGHLIGHT_PROMISES = new Map>(); const sourceFetcherIds = new WeakMap, number>(); let nextSourceFetcherId = 1; -/** 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); - } - } -} - /** Summarize rendered diff lines without serializing whole arrays into the cache key. */ function lineSetFingerprint(lines: string[] | undefined) { let totalChars = 0; @@ -118,7 +99,6 @@ function commitHighlightResult( SHARED_HIGHLIGHT_PROMISES.delete(cacheKey); SHARED_HIGHLIGHTED_DIFF_CACHE.set(cacheKey, result); - enforceCacheLimit(); return true; } @@ -128,6 +108,9 @@ function ensureHighlightedDiffLoaded( theme: AppTheme, cacheKey = highlightedDiffCacheKey(theme, file), ) { + // Viewport prefetch calls this for every file in its halo on each scroll, so this read is also + // what keeps the files around the viewport at the recent end of the cache while files entering + // the halo evict older ones. const cached = SHARED_HIGHLIGHTED_DIFF_CACHE.get(cacheKey); if (cached) { return Promise.resolve(cached); @@ -180,7 +163,9 @@ function resolveHighlightedSnapshot({ return highlighted; } - return SHARED_HIGHLIGHTED_DIFF_CACHE.get(appearanceCacheKey) ?? null; + // Peek rather than read: render stays side-effect free, and the layout effect below refreshes + // recency for this same key during commit. + return SHARED_HIGHLIGHTED_DIFF_CACHE.peek(appearanceCacheKey) ?? null; } /** Resolve highlighted diff content with shared caching and background prefetch support. */ From a696ee4123ee68f1b7f5f4242c50cde740881913 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 18:08:55 +0000 Subject: [PATCH 2/5] perf(diff): budget the highlight cache by lines, not file count The cache counted 40 entries while the prefetch window is measured in rows, so the two budgets could not agree. A 60-row viewport over a changeset of small files puts ~54 files in the window against 40 slots, so every scroll step evicted files it was about to request again. At the other end, 40 slots of a 2400-line diff retains ~130MB. Count highlighted diff lines instead. Highlighted nodes cost ~1.4KB per line measured on real TSX, so 12000 lines is ~17MB whatever the shape of the changeset, and the window - about seven viewport heights of rows - now always fits with room to scroll back. Simulating a scroll down and back over 200 files drops re-highlights from 4692 to 0 in the small-file case and from 170 to 0 in the common one. A result larger than the whole budget is still cached alone, so the file being read stays highlighted even when a single generated file pushes its neighbors out. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P2q34GvqwwMLbnBV7hjBB4 --- .changeset/highlight-cache-line-budget.md | 5 ++ src/ui/diff/highlightedDiffCache.test.ts | 78 ++++++++++++++++------- src/ui/diff/highlightedDiffCache.ts | 67 ++++++++++++------- 3 files changed, 105 insertions(+), 45 deletions(-) create mode 100644 .changeset/highlight-cache-line-budget.md diff --git a/.changeset/highlight-cache-line-budget.md b/.changeset/highlight-cache-line-budget.md new file mode 100644 index 000000000..5256b2b07 --- /dev/null +++ b/.changeset/highlight-cache-line-budget.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Budget the syntax highlighting cache by lines instead of file count, so reviews of many small files stop re-highlighting as you scroll and reviews of very large files stay within a bounded memory footprint. diff --git a/src/ui/diff/highlightedDiffCache.test.ts b/src/ui/diff/highlightedDiffCache.test.ts index f7198ce1e..4d803619e 100644 --- a/src/ui/diff/highlightedDiffCache.test.ts +++ b/src/ui/diff/highlightedDiffCache.test.ts @@ -2,17 +2,20 @@ import { describe, expect, test } from "bun:test"; import type { HighlightedDiffCode } from "./diffRows"; import { createHighlightedDiffCache } from "./highlightedDiffCache"; -/** Build a distinguishable cache value; identity is all these tests compare. */ -function createTestHighlightedDiffCode(): HighlightedDiffCode { - return { deletionLines: [], additionLines: [] }; +/** Build a result retaining a known line count; identity is what these tests compare. */ +function createTestHighlightedDiffCode(lines: number): HighlightedDiffCode { + return { + deletionLines: Array.from({ length: lines }, () => undefined), + additionLines: [], + }; } describe("highlighted diff cache", () => { test("evicts the least recently used entry rather than the oldest highlight", () => { - const cache = createHighlightedDiffCache(2); - const onScreen = createTestHighlightedDiffCode(); - const scrolledPast = createTestHighlightedDiffCode(); - const prefetched = createTestHighlightedDiffCode(); + const cache = createHighlightedDiffCache(20); + const onScreen = createTestHighlightedDiffCode(10); + const scrolledPast = createTestHighlightedDiffCode(10); + const prefetched = createTestHighlightedDiffCode(10); cache.set("on-screen", onScreen); cache.set("scrolled-past", scrolledPast); @@ -27,9 +30,9 @@ describe("highlighted diff cache", () => { }); test("peeking does not protect an entry from eviction", () => { - const cache = createHighlightedDiffCache(1); - const first = createTestHighlightedDiffCode(); - const second = createTestHighlightedDiffCode(); + const cache = createHighlightedDiffCache(10); + const first = createTestHighlightedDiffCode(10); + const second = createTestHighlightedDiffCode(10); cache.set("first", first); expect(cache.peek("first")).toBe(first); @@ -39,25 +42,54 @@ describe("highlighted diff cache", () => { expect(cache.peek("second")).toBe(second); }); - test("re-storing a key refreshes recency without growing past the budget", () => { - const cache = createHighlightedDiffCache(2); - const replaced = createTestHighlightedDiffCode(); - const kept = createTestHighlightedDiffCode(); - const added = createTestHighlightedDiffCode(); + test("holds far more small files than large ones under the same budget", () => { + const cache = createHighlightedDiffCache(100); - cache.set("reloaded", createTestHighlightedDiffCode()); + // A window of one-line fixes is what a lint or import sweep looks like. + for (let index = 0; index < 50; index += 1) { + cache.set(`small-${index}`, createTestHighlightedDiffCode(2)); + } + expect(cache.peek("small-0")).toBeDefined(); + expect(cache.peek("small-49")).toBeDefined(); + + // The same count of generated files cannot fit, and the recent ones win. + for (let index = 0; index < 50; index += 1) { + cache.set(`large-${index}`, createTestHighlightedDiffCode(60)); + } + expect(cache.peek("large-49")).toBeDefined(); + expect(cache.peek("large-0")).toBeUndefined(); + expect(cache.peek("small-0")).toBeUndefined(); + }); + + test("keeps a result larger than the whole budget rather than dropping it", () => { + const cache = createHighlightedDiffCache(100); + const neighbor = createTestHighlightedDiffCode(50); + const generated = createTestHighlightedDiffCode(5000); + + cache.set("neighbor", neighbor); + cache.set("generated", generated); + + expect(cache.peek("generated")).toBe(generated); + expect(cache.peek("neighbor")).toBeUndefined(); + }); + + test("releases the budget a replaced result was holding", () => { + const cache = createHighlightedDiffCache(100); + const reloaded = createTestHighlightedDiffCode(4); + const kept = createTestHighlightedDiffCode(40); + + // A file whose diff shrinks between reloads must not keep charging its old size. + cache.set("reloaded", createTestHighlightedDiffCode(90)); + cache.set("reloaded", reloaded); cache.set("kept", kept); - cache.set("reloaded", replaced); - cache.set("added", added); - expect(cache.peek("reloaded")).toBe(replaced); - expect(cache.peek("added")).toBe(added); - expect(cache.peek("kept")).toBeUndefined(); + expect(cache.peek("reloaded")).toBe(reloaded); + expect(cache.peek("kept")).toBe(kept); }); - test("keeps at least one entry when given a degenerate budget", () => { + test("keeps one entry when given a degenerate budget", () => { const cache = createHighlightedDiffCache(0); - const only = createTestHighlightedDiffCode(); + const only = createTestHighlightedDiffCode(5); cache.set("only", only); diff --git a/src/ui/diff/highlightedDiffCache.ts b/src/ui/diff/highlightedDiffCache.ts index 5ee4db784..ab65081a2 100644 --- a/src/ui/diff/highlightedDiffCache.ts +++ b/src/ui/diff/highlightedDiffCache.ts @@ -1,13 +1,15 @@ import type { HighlightedDiffCode } from "./diffRows"; /** - * Maximum cached highlight results. + * Cache budget, counted in highlighted diff lines. * - * Highlighted HAST nodes and their flattened render spans are expensive enough that a whole-review - * cache can dominate memory while navigating large changesets. Keep a viewport-local working set - * instead of retaining every file the user has visited in the current review. + * Highlighted HAST nodes and their flattened render spans cost roughly 1.4KB per line, so a budget + * in lines bounds memory directly where a file count could not: 40 files of a 2400-line diff is + * ~130MB, while 40 single-line fixes is a rounding error. Lines are also the unit the prefetch + * window is measured in, so the files it warms always fit — a window spans about seven viewport + * heights of rows, leaving this budget an order of magnitude of headroom for scrolling back. */ -const MAX_HIGHLIGHTED_DIFF_CACHE_ENTRIES = 40; +const MAX_HIGHLIGHTED_DIFF_CACHE_LINES = 12_000; export interface HighlightedDiffCache { /** Read one result and mark it most recently used. */ @@ -18,24 +20,38 @@ export interface HighlightedDiffCache { set: (key: string, value: HighlightedDiffCode) => void; } +interface HighlightedDiffCacheEntry { + lines: number; + value: HighlightedDiffCode; +} + +/** Count the highlighted lines one result retains, across both diff sides. */ +function highlightedLineCount(value: HighlightedDiffCode) { + return value.deletionLines.length + value.additionLines.length; +} + /** - * Holds highlight results under a bounded least-recently-used budget. + * Holds highlight results under a bounded least-recently-used line budget. * * Reads refresh recency, so eviction drops the files the review has stopped touching rather than - * the ones highlighted longest ago. Viewport prefetch re-reads its whole halo on every scroll, so - * the files on screen stay resident while files ahead of them are warmed. A halo wider than the - * budget still thrashes; that is a prefetch sizing question, not an eviction-order one. + * the ones highlighted longest ago. Viewport prefetch re-reads its whole window on every scroll, + * which keeps the files on screen resident while files ahead of them are warmed. + * + * A result larger than the whole budget is still cached, alone: the file being read must stay + * highlighted, so a single generated file can push its neighbors out. That trade is the point of + * budgeting memory rather than file count. */ export function createHighlightedDiffCache( - maxEntries = MAX_HIGHLIGHTED_DIFF_CACHE_ENTRIES, + maxLines = MAX_HIGHLIGHTED_DIFF_CACHE_LINES, ): HighlightedDiffCache { - const entries = new Map(); - const budget = Math.max(1, Math.floor(maxEntries)); + const entries = new Map(); + const budget = Math.max(1, Math.floor(maxLines)); + let cachedLines = 0; /** Move one key to the most-recently-used end of Map iteration order. */ - const touch = (key: string, value: HighlightedDiffCode) => { + const touch = (key: string, entry: HighlightedDiffCacheEntry) => { entries.delete(key); - entries.set(key, value); + entries.set(key, entry); }; return { @@ -46,25 +62,32 @@ export function createHighlightedDiffCache( } touch(key, entry); - return entry; + return entry.value; }, peek(key) { - return entries.get(key); + return entries.get(key)?.value; }, set(key, value) { - touch(key, value); + cachedLines -= entries.get(key)?.lines ?? 0; + + const lines = highlightedLineCount(value); + touch(key, { lines, value }); + cachedLines += lines; - // Map iteration order is insertion order and every read re-inserts, so the first keys are - // the least recently used. - while (entries.size > budget) { - const leastRecentlyUsed = entries.keys().next().value; + // Map iteration order is insertion order and every read re-inserts, so the first keys are the + // least recently used. Stop at one entry so the result just stored survives its own eviction + // pass even when it alone exceeds the budget. + while (cachedLines > budget && entries.size > 1) { + const leastRecentlyUsed = entries.entries().next().value; if (leastRecentlyUsed === undefined) { return; } - entries.delete(leastRecentlyUsed); + const [evictedKey, evictedEntry] = leastRecentlyUsed; + entries.delete(evictedKey); + cachedLines -= evictedEntry.lines; } }, }; From ec69971bf413e2d496b70b15c6d9b52ea17530c3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 19:14:00 +0000 Subject: [PATCH 3/5] fix(diff): size the highlight cache to hold a generated file and its neighbors A 12000-line budget was smaller than the diff of a regenerated lockfile, and an entry too large for the budget evicts its own neighbors, which then evict it back on the next scroll. Simulating a review of 20 source files around one 20000-row lockfile turned 0 re-highlights into 6179, each one a 40000-line job on the serialized highlight queue - worse than the file-count cap this replaced. Raise the budget to 60000 lines so that changeset fits whole. Every scenario in the simulation now reaches 0 re-highlights, against 20 to 4692 under the old cap. Worst case is ~84MB at the measured 1.4KB per line, and lower in practice because the files that approach the budget are generated output whose lines carry far fewer spans than the source that rate was measured on. Files whose diff exceeds the budget on their own still thrash. Bounding that needs a limit on what gets highlighted at all, not a cache change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P2q34GvqwwMLbnBV7hjBB4 --- src/ui/diff/highlightedDiffCache.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/ui/diff/highlightedDiffCache.ts b/src/ui/diff/highlightedDiffCache.ts index ab65081a2..c3177957d 100644 --- a/src/ui/diff/highlightedDiffCache.ts +++ b/src/ui/diff/highlightedDiffCache.ts @@ -7,9 +7,16 @@ import type { HighlightedDiffCode } from "./diffRows"; * in lines bounds memory directly where a file count could not: 40 files of a 2400-line diff is * ~130MB, while 40 single-line fixes is a rounding error. Lines are also the unit the prefetch * window is measured in, so the files it warms always fit — a window spans about seven viewport - * heights of rows, leaving this budget an order of magnitude of headroom for scrolling back. + * heights of rows, a rounding error against this budget. + * + * The ceiling is set high enough to hold a generated file alongside the source files around it. + * Below that, an entry too large for the budget evicts its own neighbors, which then evict it back + * on the next scroll, re-highlighting tens of thousands of lines per step. Worst case is ~84MB at + * the measured rate, and well under that in practice: the files that approach the budget are + * lockfiles and generated output, whose lines carry far fewer spans than the dense source the rate + * was measured on. */ -const MAX_HIGHLIGHTED_DIFF_CACHE_LINES = 12_000; +const MAX_HIGHLIGHTED_DIFF_CACHE_LINES = 60_000; export interface HighlightedDiffCache { /** Read one result and mark it most recently used. */ From c282a21e52714c848834e09ace19a5add5962987 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 19:55:25 +0000 Subject: [PATCH 4/5] perf(diff): stop highlighting diffs past 10000 lines A file whose highlight is too large for the cache budget evicts the files around it and is evicted back on the next scroll. Raising the budget moved that threshold but could not remove it, because no cache holds an entry larger than itself alongside anything else. Skip highlighting past 10000 diff lines instead, checked before the source read so an oversized file costs neither I/O nor a slot on the serialized highlight queue. Diffs that size are generated output, where color earns little and the wait is longest; they now paint as plain rows immediately. With no entry able to exceed the budget, every simulated review reaches zero re-highlights, including the 40000-row generated file that still thrashed at 12179 before this. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P2q34GvqwwMLbnBV7hjBB4 --- .../skip-generated-file-highlighting.md | 5 ++ src/ui/diff/diffRows.test.ts | 46 +++++++++++++++++++ src/ui/diff/diffRows.ts | 35 ++++++++++++++ src/ui/diff/highlightedDiffCache.ts | 9 ++-- 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 .changeset/skip-generated-file-highlighting.md diff --git a/.changeset/skip-generated-file-highlighting.md b/.changeset/skip-generated-file-highlighting.md new file mode 100644 index 000000000..5b8edc6c7 --- /dev/null +++ b/.changeset/skip-generated-file-highlighting.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Render diffs larger than 10,000 lines as plain rows instead of syntax highlighting them, so a regenerated lockfile appears immediately and stops delaying color on the files around it. diff --git a/src/ui/diff/diffRows.test.ts b/src/ui/diff/diffRows.test.ts index 78b25eddf..b1d3f7742 100644 --- a/src/ui/diff/diffRows.test.ts +++ b/src/ui/diff/diffRows.test.ts @@ -5,8 +5,10 @@ import type { DiffFile } from "../../core/types"; import { buildSplitRows, buildStackRows, + highlightedDiffLineCount, loadHighlightedDiff, loadHighlightedSourceLines, + shouldHighlightDiff, spansForHighlightedSourceLine, type DiffRow, } from "./diffRows"; @@ -50,6 +52,26 @@ function createDiffFile(): DiffFile { }; } +/** Build an added-file diff whose every line lands on the addition side, as generated output does. */ +function createGeneratedFileDiff(contents: string): DiffFile { + const metadata = parseDiffFromFile( + { name: "bun.lock", contents: "", cacheKey: "generated:before" }, + { name: "bun.lock", contents, cacheKey: "generated:after" }, + { context: 3 }, + true, + ); + + return { + id: "generated", + path: "bun.lock", + patch: "", + language: "json", + stats: { additions: metadata.additionLines.length, deletions: 0 }, + metadata, + agent: null, + }; +} + const ELIXIR_HEREDOC_BEFORE = `defmodule Repro do @doc """ Line one. @@ -169,6 +191,30 @@ describe("Pierre diff rows", () => { ).toBe(true); }); + test("renders a generated-scale diff as plain rows instead of highlighting it", async () => { + const generatedLines = Array.from( + { length: 12_000 }, + (_, index) => ` "package-${index}": "npm:package-${index}@1.0.${index}",`, + ).join("\n"); + const file = createGeneratedFileDiff(`{\n${generatedLines}\n}\n`); + const theme = resolveTheme("github-dark-default", null); + + expect(shouldHighlightDiff(file)).toBe(false); + expect(highlightedDiffLineCount(file.metadata)).toBeGreaterThan(10_000); + + const highlighted = await loadHighlightedDiff(file, theme); + + expect(highlighted.deletionLines).toHaveLength(0); + expect(highlighted.additionLines).toHaveLength(0); + + // Plain rows still carry the diff itself, so the file stays reviewable without color. + const rows = buildStackRows(file, highlighted, theme); + expect(rows.some((row) => row.type === "stack-line")).toBe(true); + + // A source file of ordinary size is unaffected. + expect(shouldHighlightDiff(createDiffFile())).toBe(true); + }); + test("uses full source to keep partial Elixir hunks inside the correct heredoc state", async () => { const sourceFetcher = createTestSourceFetcher((side) => side === "old" ? ELIXIR_HEREDOC_BEFORE : ELIXIR_HEREDOC_AFTER, diff --git a/src/ui/diff/diffRows.ts b/src/ui/diff/diffRows.ts index 19077eefd..d670ad607 100644 --- a/src/ui/diff/diffRows.ts +++ b/src/ui/diff/diffRows.ts @@ -643,11 +643,46 @@ function renderHighlightedDiff( }); } +/** + * Largest diff, in lines across both sides, that is worth syntax highlighting. + * + * Past this size the work stops paying for itself twice over: the job occupies the serialized + * highlight queue for seconds while nothing else can be colorized, and the result is too large for + * the shared cache to hold beside the files around it, so it evicts its neighbors and is evicted + * back on the next scroll. Diffs this big are generated output — lockfiles, snapshots, vendored + * bundles — where color earns little. They render as plain rows instead, immediately. + * + * Keep this at or below the cache budget in `highlightedDiffCache.ts`; that is what guarantees no + * single entry can push the rest of the working set out. + */ +const MAX_HIGHLIGHTED_DIFF_LINES = 10_000; + +/** Shared plain-rows result. Read-only, so one instance can back every skipped file. */ +const UNHIGHLIGHTED_DIFF: HighlightedDiffCode = Object.freeze({ + deletionLines: [], + additionLines: [], +}); + +/** Count the diff lines one file retains when highlighted, across both sides. */ +export function highlightedDiffLineCount(metadata: FileDiffMetadata) { + return (metadata.deletionLines?.length ?? 0) + (metadata.additionLines?.length ?? 0); +} + +/** Return whether one file's diff is small enough that highlighting it is worth the work. */ +export function shouldHighlightDiff(file: DiffFile) { + return highlightedDiffLineCount(file.metadata) <= MAX_HIGHLIGHTED_DIFF_LINES; +} + /** Highlight a diff file and return just the rendered line trees the UI needs. */ export async function loadHighlightedDiff( file: DiffFile, theme: HighlightThemeInput = "dark", ): Promise { + // Checked before the source read so an oversized file costs neither I/O nor queue time. + if (!shouldHighlightDiff(file)) { + return UNHIGHLIGHTED_DIFF; + } + const sourcePlan = await loadSourceBackedHighlightPlan(file); try { diff --git a/src/ui/diff/highlightedDiffCache.ts b/src/ui/diff/highlightedDiffCache.ts index c3177957d..8c1458093 100644 --- a/src/ui/diff/highlightedDiffCache.ts +++ b/src/ui/diff/highlightedDiffCache.ts @@ -9,12 +9,9 @@ import type { HighlightedDiffCode } from "./diffRows"; * window is measured in, so the files it warms always fit — a window spans about seven viewport * heights of rows, a rounding error against this budget. * - * The ceiling is set high enough to hold a generated file alongside the source files around it. - * Below that, an entry too large for the budget evicts its own neighbors, which then evict it back - * on the next scroll, re-highlighting tens of thousands of lines per step. Worst case is ~84MB at - * the measured rate, and well under that in practice: the files that approach the budget are - * lockfiles and generated output, whose lines carry far fewer spans than the dense source the rate - * was measured on. + * The budget holds several of the largest highlightable files at once. `MAX_HIGHLIGHTED_DIFF_LINES` + * caps any single entry well below it, which is what keeps one file from evicting its own neighbors + * and being evicted back on the next scroll. Worst case is ~84MB at the measured rate. */ const MAX_HIGHLIGHTED_DIFF_CACHE_LINES = 60_000; From 7f74a05085c77c164ae3f3eabf9787ce2477dac1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 20:38:34 +0000 Subject: [PATCH 5/5] fix(diff): charge cache bookkeeping so empty results age out A skipped or failed highlight retains no lines, so it weighed nothing against the line budget and never reached the eviction loop. Its key and entry object stayed for the process lifetime, and a watch session reloading an oversized file mints a fresh key per reload, so those accumulated without bound. Charge every entry its retained lines plus a fixed overhead, which is also closer to what a mostly-key entry actually costs. Reported by Greptile on #754. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01P2q34GvqwwMLbnBV7hjBB4 --- src/ui/diff/highlightedDiffCache.test.ts | 20 +++++++++++++--- src/ui/diff/highlightedDiffCache.ts | 30 +++++++++++++++++------- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/ui/diff/highlightedDiffCache.test.ts b/src/ui/diff/highlightedDiffCache.test.ts index 4d803619e..f2d360dfc 100644 --- a/src/ui/diff/highlightedDiffCache.test.ts +++ b/src/ui/diff/highlightedDiffCache.test.ts @@ -12,7 +12,8 @@ function createTestHighlightedDiffCode(lines: number): HighlightedDiffCode { describe("highlighted diff cache", () => { test("evicts the least recently used entry rather than the oldest highlight", () => { - const cache = createHighlightedDiffCache(20); + // Three 10-line results cost 18 each with per-entry overhead, so two fit and a third evicts. + const cache = createHighlightedDiffCache(40); const onScreen = createTestHighlightedDiffCode(10); const scrolledPast = createTestHighlightedDiffCode(10); const prefetched = createTestHighlightedDiffCode(10); @@ -30,7 +31,7 @@ describe("highlighted diff cache", () => { }); test("peeking does not protect an entry from eviction", () => { - const cache = createHighlightedDiffCache(10); + const cache = createHighlightedDiffCache(20); const first = createTestHighlightedDiffCode(10); const second = createTestHighlightedDiffCode(10); @@ -43,7 +44,7 @@ describe("highlighted diff cache", () => { }); test("holds far more small files than large ones under the same budget", () => { - const cache = createHighlightedDiffCache(100); + const cache = createHighlightedDiffCache(600); // A window of one-line fixes is what a lint or import sweep looks like. for (let index = 0; index < 50; index += 1) { @@ -87,6 +88,19 @@ describe("highlighted diff cache", () => { expect(cache.peek("kept")).toBe(kept); }); + test("reclaims entries that retain no lines at all", () => { + const cache = createHighlightedDiffCache(100); + + // Diffs past the highlight ceiling cache an empty result. A watch session reloading one + // produces a fresh key per reload, so these have to age out like any other entry. + for (let index = 0; index < 200; index += 1) { + cache.set(`skipped-${index}`, createTestHighlightedDiffCode(0)); + } + + expect(cache.peek("skipped-199")).toBeDefined(); + expect(cache.peek("skipped-0")).toBeUndefined(); + }); + test("keeps one entry when given a degenerate budget", () => { const cache = createHighlightedDiffCache(0); const only = createTestHighlightedDiffCode(5); diff --git a/src/ui/diff/highlightedDiffCache.ts b/src/ui/diff/highlightedDiffCache.ts index 8c1458093..00b7176a9 100644 --- a/src/ui/diff/highlightedDiffCache.ts +++ b/src/ui/diff/highlightedDiffCache.ts @@ -24,8 +24,17 @@ export interface HighlightedDiffCache { set: (key: string, value: HighlightedDiffCode) => void; } +/** + * Bookkeeping charged to every entry, in line-equivalents. + * + * A skipped or failed highlight retains no lines but still holds a cache key and an entry object. + * Charged nothing, those never reach the eviction loop, so a long watch session reloading an + * oversized file would accumulate keys the budget could never reclaim. + */ +const ENTRY_OVERHEAD_LINES = 8; + interface HighlightedDiffCacheEntry { - lines: number; + cost: number; value: HighlightedDiffCode; } @@ -34,6 +43,11 @@ function highlightedLineCount(value: HighlightedDiffCode) { return value.deletionLines.length + value.additionLines.length; } +/** Charge one result its retained lines plus per-entry bookkeeping. */ +function entryCost(value: HighlightedDiffCode) { + return highlightedLineCount(value) + ENTRY_OVERHEAD_LINES; +} + /** * Holds highlight results under a bounded least-recently-used line budget. * @@ -50,7 +64,7 @@ export function createHighlightedDiffCache( ): HighlightedDiffCache { const entries = new Map(); const budget = Math.max(1, Math.floor(maxLines)); - let cachedLines = 0; + let cachedCost = 0; /** Move one key to the most-recently-used end of Map iteration order. */ const touch = (key: string, entry: HighlightedDiffCacheEntry) => { @@ -74,16 +88,16 @@ export function createHighlightedDiffCache( }, set(key, value) { - cachedLines -= entries.get(key)?.lines ?? 0; + cachedCost -= entries.get(key)?.cost ?? 0; - const lines = highlightedLineCount(value); - touch(key, { lines, value }); - cachedLines += lines; + const cost = entryCost(value); + touch(key, { cost, value }); + cachedCost += cost; // Map iteration order is insertion order and every read re-inserts, so the first keys are the // least recently used. Stop at one entry so the result just stored survives its own eviction // pass even when it alone exceeds the budget. - while (cachedLines > budget && entries.size > 1) { + while (cachedCost > budget && entries.size > 1) { const leastRecentlyUsed = entries.entries().next().value; if (leastRecentlyUsed === undefined) { return; @@ -91,7 +105,7 @@ export function createHighlightedDiffCache( const [evictedKey, evictedEntry] = leastRecentlyUsed; entries.delete(evictedKey); - cachedLines -= evictedEntry.lines; + cachedCost -= evictedEntry.cost; } }, };