From d065902404dee5a609767b5c6c00951608a396d0 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Wed, 19 Aug 2026 01:32:26 +0200 Subject: [PATCH] refactor(feed): cap the read-ahead lookahead and extract it to a tested helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. The viewport-scaled lookahead was unbounded, so a tall window prefetched several feed pages on every mount / filter change before any scroll — wasted work on the app's heaviest query. Clamp it to MAX_LOOKAHEAD_ROWS, and move the policy into a pure readAhead.ts (readAheadLookahead / shouldPrefetchNext) with unit tests covering the floor, the scaling, the cap boundary, and the fetch trigger — the core of the feed-scroll fix now has coverage instead of only manual E2E. --- frontend/src/components/VirtualGrid.tsx | 26 +++++--------- frontend/src/components/readAhead.test.ts | 44 +++++++++++++++++++++++ frontend/src/components/readAhead.ts | 30 ++++++++++++++++ 3 files changed, 82 insertions(+), 18 deletions(-) create mode 100644 frontend/src/components/readAhead.test.ts create mode 100644 frontend/src/components/readAhead.ts diff --git a/frontend/src/components/VirtualGrid.tsx b/frontend/src/components/VirtualGrid.tsx index c5e0a7ed..6add3475 100644 --- a/frontend/src/components/VirtualGrid.tsx +++ b/frontend/src/components/VirtualGrid.tsx @@ -8,6 +8,7 @@ import { type ReactNode, } from "react"; import { useVirtualizer } from "@tanstack/react-virtual"; +import { shouldPrefetchNext } from "./readAhead"; // The virtualization engine, lifted out of VirtualFeed so any long client- or server-side list can // reuse it (the feed's video grid, the channel manager's card/row views). It owns ONLY the layout @@ -23,17 +24,6 @@ import { useVirtualizer } from "@tanstack/react-virtual"; // (grid-cols-[repeat(auto-fill,minmax(260px,1fr))] with a gap-4 / 1rem gutter). const GRID_GAP = 16; const LIST_GAP = 4; -// Infinite-scroll read-ahead. The next page is pulled once the last rendered row comes within -// `lookahead` rows of the loaded end, where lookahead = max(PREFETCH_ROWS, rendered-rows × -// PREFETCH_VIEWPORTS). Sizing it to the number of rows the viewport currently renders makes it adapt -// to window height / column count / row height, and keeps ~PREFETCH_VIEWPORTS screenfuls buffered -// below the fold. The fixed floor is the finite-list / first-paint case. -const PREFETCH_ROWS = 4; -// Keep this many screenfuls of rows loaded ahead of the fold. A fast free-spin flick (MX Master & -// co.) out-scrolls a mere 4-row lookahead and lands on an empty bottom, then waits out serial keyset -// round-trips — the "Load more" hangs for seconds. Buffering a couple of screens ahead means the -// content is usually already there when the user gets to it. -const PREFETCH_VIEWPORTS = 2; /** Columns that fit `width`, or 1 for the single-column layout (`minCol == null`). */ function columnsFor(minCol: number | null, width: number): number { @@ -148,17 +138,17 @@ export default function VirtualGrid({ const virtualRows = virtualizer.getVirtualItems(); - // Infinite-scroll read-ahead: once the last rendered row is within `lookahead` rows of the end, - // pull the next page. A no-op for finite lists (no fetchNextPage / hasNextPage false). Because this - // effect re-runs when a page settles (isFetchingNextPage flips false) it CHAINS — refilling the - // buffer page by page as the user consumes it, so a fast scroll reaches the loaded end far less - // often. See PREFETCH_VIEWPORTS above for why the lookahead scales with the rendered row count. + // Infinite-scroll read-ahead: pull the next page once the last rendered row is within the + // read-ahead window of the loaded end (see readAhead.ts — the window scales with the rendered row + // count, clamped so a tall window doesn't prefetch unboundedly). A no-op for finite lists (no + // fetchNextPage / hasNextPage false). Because this effect re-runs when a page settles + // (isFetchingNextPage flips false) it CHAINS — refilling the buffer as the user consumes it, so a + // fast scroll reaches the loaded end far less often. useEffect(() => { if (!fetchNextPage || !hasNextPage || isFetchingNextPage) return; const last = virtualRows[virtualRows.length - 1]; if (!last) return; - const lookahead = Math.max(PREFETCH_ROWS, virtualRows.length * PREFETCH_VIEWPORTS); - if (last.index >= rows.length - 1 - lookahead) { + if (shouldPrefetchNext(last.index, rows.length, virtualRows.length)) { fetchNextPage(); } }, [virtualRows, rows.length, hasNextPage, isFetchingNextPage, fetchNextPage]); diff --git a/frontend/src/components/readAhead.test.ts b/frontend/src/components/readAhead.test.ts new file mode 100644 index 00000000..83fa3c54 --- /dev/null +++ b/frontend/src/components/readAhead.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import { + MAX_LOOKAHEAD_ROWS, + PREFETCH_ROWS, + PREFETCH_VIEWPORTS, + readAheadLookahead, + shouldPrefetchNext, +} from "./readAhead"; + +describe("readAheadLookahead", () => { + it("floors at PREFETCH_ROWS when nothing (or little) is rendered", () => { + expect(readAheadLookahead(0)).toBe(PREFETCH_ROWS); + expect(readAheadLookahead(1)).toBe(PREFETCH_ROWS); // 1*2=2 < floor 4 + }); + + it("scales with the rendered row count for a normal viewport", () => { + expect(readAheadLookahead(10)).toBe(10 * PREFETCH_VIEWPORTS); // 20 + expect(readAheadLookahead(15)).toBe(30); + }); + + it("caps at MAX_LOOKAHEAD_ROWS so a tall window doesn't prefetch unboundedly", () => { + // 30 rendered rows would be 60 uncapped — the cap is what bounds per-mount feed prefetch. + expect(readAheadLookahead(30)).toBe(MAX_LOOKAHEAD_ROWS); + expect(readAheadLookahead(1000)).toBe(MAX_LOOKAHEAD_ROWS); + }); +}); + +describe("shouldPrefetchNext", () => { + it("fetches once the last rendered row enters the read-ahead window", () => { + // 100 rows loaded, viewport renders 10 → lookahead 20. Fetch when lastIndex >= 100-1-20 = 79. + expect(shouldPrefetchNext(78, 100, 10)).toBe(false); + expect(shouldPrefetchNext(79, 100, 10)).toBe(true); + expect(shouldPrefetchNext(99, 100, 10)).toBe(true); + }); + + it("fetches eagerly while fewer rows are loaded than the lookahead (early pages)", () => { + // Only 15 rows loaded, lookahead 20 → the whole list is inside the window. + expect(shouldPrefetchNext(8, 15, 10)).toBe(true); + }); + + it("does not fetch near the top of a long loaded list", () => { + expect(shouldPrefetchNext(12, 500, 10)).toBe(false); + }); +}); diff --git a/frontend/src/components/readAhead.ts b/frontend/src/components/readAhead.ts new file mode 100644 index 00000000..701adcdc --- /dev/null +++ b/frontend/src/components/readAhead.ts @@ -0,0 +1,30 @@ +// Infinite-scroll read-ahead policy for VirtualGrid, extracted as a pure function so the decision is +// unit-testable without a layout engine. +// +// The next page is pulled once the last rendered row comes within `lookahead` rows of the loaded end, +// where lookahead scales with the number of rows the viewport currently renders (so it adapts to +// window height / column count / row height) but is clamped to a floor and a ceiling: +// - floor PREFETCH_ROWS: the finite-list / first-paint case where nothing is rendered yet. +// - ceiling MAX_LOOKAHEAD_ROWS: a tall window renders many rows, and an uncapped lookahead would +// eagerly prefetch several pages on every mount / filter change — wasted work on the app's +// heaviest query (the feed). The cap bounds that while still buffering a couple of screens ahead. +export const PREFETCH_ROWS = 4; +export const PREFETCH_VIEWPORTS = 2; +export const MAX_LOOKAHEAD_ROWS = 40; + +/** How many rows ahead of the last rendered row to keep buffered, for a viewport rendering + * `renderedRows` rows. Clamped to [PREFETCH_ROWS, MAX_LOOKAHEAD_ROWS]. */ +export function readAheadLookahead(renderedRows: number): number { + return Math.min(MAX_LOOKAHEAD_ROWS, Math.max(PREFETCH_ROWS, renderedRows * PREFETCH_VIEWPORTS)); +} + +/** Whether to fetch the next page: true once the last rendered row (`lastIndex`) is within the + * read-ahead window of the loaded end (`rowCount` total loaded rows). Because the caller re-runs + * this as each page settles, it chains — refilling the buffer as the user consumes it. */ +export function shouldPrefetchNext( + lastIndex: number, + rowCount: number, + renderedRows: number, +): boolean { + return lastIndex >= rowCount - 1 - readAheadLookahead(renderedRows); +}