From 25b07941815ca6934683d6c3e9c1f8a5e612d7ab Mon Sep 17 00:00:00 2001 From: Adamulek123 Date: Sat, 12 Sep 2026 13:28:28 +0200 Subject: [PATCH 1/2] perf(web): defer PR partition reads until armed --- .../pullRequest/pullRequestList.logic.test.ts | 86 +++++++++++++++++ .../pullRequest/pullRequestList.logic.ts | 68 ++++++++++++++ apps/web/src/routes/_chat.pull-requests.tsx | 94 ++++++++++++------- 3 files changed, 212 insertions(+), 36 deletions(-) diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts index a90022ad72c9..8f18b1a79a83 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from "vite-plus/test"; import { filterPullRequestsByInvolvement, findScopedProject, + buildPullRequestPartitionTargets, mergePullRequestLists, pullRequestEntryKey, pullRequestEnvironmentSetKey, @@ -1496,3 +1497,88 @@ describe("the priority groups against a paginated feed", () => { ).toEqual([6123]); }); }); + +describe("the priority groups' deferred reads", () => { + const base = { + wanted: true, + armed: true, + hasBaselineRows: true, + environments: [{ environmentId: ENV_1 }], + state: "open" as const, + limit: 99, + }; + + it("stays unread until armed, so deleting the guard fails without it", () => { + expect(buildPullRequestPartitionTargets({ ...base, armed: false })).toEqual({ + authored: [], + reviewing: [], + }); + }); + + it("stays unread without baseline rows or when unwanted", () => { + expect(buildPullRequestPartitionTargets({ ...base, hasBaselineRows: false })).toEqual({ + authored: [], + reviewing: [], + }); + expect(buildPullRequestPartitionTargets({ ...base, wanted: false })).toEqual({ + authored: [], + reviewing: [], + }); + }); + + it("builds one server-filtered read per involvement once armed", () => { + const targets = buildPullRequestPartitionTargets(base); + expect(targets.authored).toHaveLength(1); + expect(targets.reviewing).toHaveLength(1); + expect(targets.authored[0]).toMatchObject({ + environmentId: ENV_1, + input: { state: "open", involvement: "authored", limit: 99 }, + }); + expect(targets.reviewing[0]).toMatchObject({ + environmentId: ENV_1, + input: { state: "open", involvement: "reviewing", limit: 99 }, + }); + }); + + it("fans scoped project, host, and filters out to every environment in feed field order", () => { + const filters = { draft: "hide" } as const; + const targets = buildPullRequestPartitionTargets({ + ...base, + environments: [{ environmentId: ENV_1 }, { environmentId: ENV_2 }], + scopedProjectId: "project-1" as ProjectId, + host: "github.com", + filters, + }); + expect(targets.authored.map((target) => target.environmentId)).toEqual([ENV_1, ENV_2]); + expect(targets.reviewing.map((target) => target.environmentId)).toEqual([ENV_1, ENV_2]); + for (const [involvement, group] of [ + ["authored", targets.authored], + ["reviewing", targets.reviewing], + ] as const) { + for (const target of group) { + expect(target.input).toEqual({ + state: "open", + involvement, + limit: 99, + projectId: "project-1", + host: "github.com", + filters, + }); + expect(Object.keys(target.input)).toEqual([ + "state", + "involvement", + "limit", + "projectId", + "host", + "filters", + ]); + expect(target.input.projectIds).toBeUndefined(); + } + } + }); + + // No column render/targets assertion: arming lives in the route's unexported column wiring + // (pointer/focus + idle callback into query atoms), which needs the full environment/query + // harness to mount; asserting the JSX prop alone would not prove a fetch, so the gating + // above carries the guarantee. +}); diff --git a/apps/web/src/components/pullRequest/pullRequestList.logic.ts b/apps/web/src/components/pullRequest/pullRequestList.logic.ts index c2fdde3d011f..98136c9a9ae4 100644 --- a/apps/web/src/components/pullRequest/pullRequestList.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestList.logic.ts @@ -631,6 +631,74 @@ export function partitionPullRequestsWithPriority ({ ...group, label: GROUP_LABELS[group.key] })); } +/** + * One environment the priority groups can be asked of, with the project split the feed uses. + * Kept structural (rather than reusing the state's query-target type) so the page's gating + * stays a pure function of its inputs and unit-testable without the query layer. + */ +export interface PullRequestPartitionEnvironment { + readonly environmentId: EnvironmentId; + readonly projectIds?: ReadonlyArray; +} + +export interface PullRequestPartitionListTarget { + readonly environmentId: EnvironmentId; + readonly input: { + readonly state: PullRequestListState; + readonly involvement: PullRequestInvolvement; + readonly limit: number; + readonly projectId?: ProjectId; + readonly projectIds?: ReadonlyArray; + readonly host?: string; + readonly filters?: PullRequestListFilters; + }; +} + +/** + * Which server-filtered reads the priority groups need. Unarmed (before engagement or idle), + * unwanted (a search re-ranks the whole list), or with no baseline rows, both stay unread and + * the groups fall back to local grouping. Built in the feed input's field order so the atoms + * stay keyed alike and the Authored/Reviewing tabs answer from this same read. + * + * The main list goes first: besides putting the visible rows on screen sooner, it proves which + * repositories the host search indexes, so an empty partition does not trigger the expensive + * per-repository fallback. With no baseline rows at all both partitions are already empty. + */ +export function buildPullRequestPartitionTargets(args: { + readonly wanted: boolean; + readonly armed: boolean; + readonly hasBaselineRows: boolean; + readonly environments: ReadonlyArray; + readonly state: PullRequestListState; + readonly limit: number; + readonly scopedProjectId?: ProjectId; + readonly host?: string; + readonly filters?: PullRequestListFilters; +}): { + readonly authored: ReadonlyArray; + readonly reviewing: ReadonlyArray; +} { + if (!args.wanted || !args.armed || !args.hasBaselineRows) { + return { authored: [], reviewing: [] }; + } + const targetsFor = ( + involvement: PullRequestInvolvement, + ): ReadonlyArray => + args.environments.map(({ environmentId, projectIds }) => ({ + environmentId, + input: { + state: args.state, + involvement, + limit: args.limit, + ...(args.scopedProjectId ? { projectId: args.scopedProjectId } : {}), + ...(projectIds ? { projectIds } : {}), + ...(args.host ? { host: args.host } : {}), + ...(args.filters ? { filters: args.filters } : {}), + }, + })); + return { authored: targetsFor("authored"), reviewing: targetsFor("reviewing") }; +} + export type PullRequestDiffStats = ReadonlyMap< string, { readonly additions: number; readonly deletions: number } diff --git a/apps/web/src/routes/_chat.pull-requests.tsx b/apps/web/src/routes/_chat.pull-requests.tsx index 2b5586a6e8e5..6f1f5e400322 100644 --- a/apps/web/src/routes/_chat.pull-requests.tsx +++ b/apps/web/src/routes/_chat.pull-requests.tsx @@ -45,6 +45,7 @@ import { import { filterPullRequestsByInvolvement, findScopedProject, + buildPullRequestPartitionTargets, collectPullRequestListFacets, groupPullRequestsByInvolvement, matchesPullRequestFilters, @@ -760,43 +761,53 @@ function PullRequestsRouteView() { // so no partitions are read for one. These are the same atoms the Authored and Reviewing tabs // ask for, so switching to either is answered from cache. const partitionsWanted = search.involvement === "all" && typedQuery.length === 0; + // The two reads below stay off the first-paint path. The feed renders from the single `all` + // read — grouped locally, or from the snapshot's held groups — and these follow once the + // reader engages (pointer or focus reaching the column, one gesture from the tabs) or the + // browser idles. A cold open therefore spends one host search to first paint instead of + // three, while a later tab switch still answers from a warmed cache. + // Cost window: a pre-arm tab switch pays one fetch per environment (the partitions were never + // warmed); post-arm the same switch is a cache hit on the atoms below, keyed like the tabs. + const [partitionsArmed, setPartitionsArmed] = useState(false); + const armPartitions = useCallback(() => setPartitionsArmed(true), []); + useEffect(() => { + if (!partitionsWanted || partitionsArmed || typeof window === "undefined") return; + if (typeof window.requestIdleCallback === "function") { + const id = window.requestIdleCallback(armPartitions, { timeout: 2000 }); + return () => window.cancelIdleCallback(id); + } + const timer = window.setTimeout(armPartitions, 1500); + return () => window.clearTimeout(timer); + }, [armPartitions, partitionsWanted, partitionsArmed]); // Built together so the two reads share one memo, and in the same field order the feed's own // input uses: the atoms are keyed by their input, so the Authored tab then reads this answer. - const partitionTargets = useMemo(() => { - // The main list goes first. Besides putting the visible rows on screen sooner, it proves - // which repositories the host search indexes, so an empty partition does not trigger the - // expensive per-repository fallback. With no rows at all both partitions are already empty. - if ( - !partitionsWanted || - baselineQuery.data === null || - baselineQuery.data.entries.length === 0 - ) { - return { authored: NO_LIST_TARGETS, reviewing: NO_LIST_TARGETS }; - } - const targetsFor = (involvement: PullRequestInvolvement) => - environmentQueries.map(({ environmentId, projectIds }) => ({ - environmentId, - input: { - state: search.state, - involvement, - limit: PAGE_SIZE, - ...(scopedProjectId ? { projectId: scopedProjectId } : {}), - ...(projectIds ? { projectIds } : {}), - ...(search.host ? { host: search.host } : {}), - ...(menuFiltered ? { filters: menuFilters } : {}), - } satisfies PullRequestListInput, - })); - return { authored: targetsFor("authored"), reviewing: targetsFor("reviewing") }; - }, [ - menuFiltered, - menuFilters, - partitionsWanted, - baselineQuery.data, - environmentQueries, - scopedProjectId, - search.host, - search.state, - ]); + // Gating lives in `buildPullRequestPartitionTargets` (unit-tested): unarmed, unwanted, or + // with no baseline rows, both stay unread and the groups fall back to local grouping. + const partitionTargets = useMemo( + () => + buildPullRequestPartitionTargets({ + wanted: partitionsWanted, + armed: partitionsArmed, + hasBaselineRows: baselineQuery.data !== null && baselineQuery.data.entries.length > 0, + environments: environmentQueries, + state: search.state, + limit: PAGE_SIZE, + ...(scopedProjectId ? { scopedProjectId } : {}), + ...(search.host ? { host: search.host } : {}), + ...(menuFiltered ? { filters: menuFilters } : {}), + }), + [ + menuFiltered, + menuFilters, + partitionsWanted, + partitionsArmed, + baselineQuery.data, + environmentQueries, + scopedProjectId, + search.host, + search.state, + ], + ); const authoredQuery = usePullRequestList(partitionTargets.authored); const reviewingQuery = usePullRequestList(partitionTargets.reviewing); // The header's refresh punches through the server's cache before re-reading; the error and @@ -1807,6 +1818,7 @@ function PullRequestsRouteView() { host: search.host, hostMenuOptions, onInvolvement: (involvement: PullRequestInvolvement) => updateListScope({ involvement }), + onPartitionsIntent: armPartitions, onState: (state: PullRequestListState) => updateListScope({ state }), onHost: (host: string | undefined) => updateListScope({ host }), searchInput, @@ -2202,6 +2214,7 @@ function PullRequestsColumn({ host, hostMenuOptions, onInvolvement, + onPartitionsIntent, onState, onHost, searchInput, @@ -2221,6 +2234,7 @@ function PullRequestsColumn({ host: string | undefined; hostMenuOptions: ReadonlyArray>; onInvolvement: (involvement: PullRequestInvolvement) => void; + onPartitionsIntent: () => void; onState: (state: PullRequestListState) => void; onHost: (host: string | undefined) => void; searchInput: ReactNode; @@ -2290,7 +2304,15 @@ function PullRequestsColumn({ return ( // Painted flat like the chat column: the inset underneath carries the chrome grain, and a // content surface that lets it show reads as a different background than every thread. -
+ // Reaching the column means its tabs and priority groups are one gesture away, so their + // deferred reads warm here rather than on first paint. The signal is deliberately broad + // (whole column): narrowing to the tab strip/scroll viewport is follow-up if profiling + // ever shows reason to narrow it. +
{/* A closed right panel leaves this column full-width, so the shared header reserves native window controls and hosts the controls strip itself: on desktop the header is a drag-region, and only a no-drag descendant wins From 764668d2b3ff66b6d2a402e0d4db6e39bc2e7f67 Mon Sep 17 00:00:00 2001 From: Adamulek123 Date: Sun, 13 Sep 2026 11:22:37 +0200 Subject: [PATCH 2/2] test(web): cover PR column intent warmers forwarding --- .../src/routes/_chat.pull-requests.test.tsx | 74 +++++++++++++++++++ apps/web/src/routes/_chat.pull-requests.tsx | 2 +- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 apps/web/src/routes/_chat.pull-requests.test.tsx diff --git a/apps/web/src/routes/_chat.pull-requests.test.tsx b/apps/web/src/routes/_chat.pull-requests.test.tsx new file mode 100644 index 000000000000..e8ed3a50d6f5 --- /dev/null +++ b/apps/web/src/routes/_chat.pull-requests.test.tsx @@ -0,0 +1,74 @@ +import { act } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { PullRequestsColumn } from "./_chat.pull-requests"; + +let renderer: ReactTestRenderer | null = null; + +beforeEach(() => { + // Same stub shape as useComposerMenuState.test.tsx: no DOM, just an event target. + // react-test-renderer needs no host nodes, but the column's effects touch + // window and IntersectionObserver, so those get minimal no-op stubs. + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("window", { addEventListener() {}, removeEventListener() {} }); + vi.stubGlobal( + "IntersectionObserver", + class { + observe() {} + unobserve() {} + disconnect() {} + }, + ); +}); + +afterEach(async () => { + await act(() => renderer?.unmount()); + renderer = null; + vi.unstubAllGlobals(); +}); + +describe("PullRequestsColumn partitions intent", () => { + it("forwards pointer and focus warmers to onPartitionsIntent", async () => { + const onPartitionsIntent = vi.fn(); + await act(() => { + renderer = create( + {}} + searchValue="" + involvement="all" + state="open" + host={undefined} + hostMenuOptions={[]} + onInvolvement={() => {}} + onPartitionsIntent={onPartitionsIntent} + onState={() => {}} + onHost={() => {}} + searchInput={null} + sortMenu={null} + filtersMenu={null} + rightPanelControl={null} + titlebarControls={null} + rightPanelOpen={false} + listBody={null} + scrollRef={{ current: null }} + />, + ); + }); + + const outer = renderer!.root.findAllByType("div")[0]!; + expect(typeof outer.props.onPointerEnter).toBe("function"); + expect(typeof outer.props.onFocusCapture).toBe("function"); + + await act(() => { + outer.props.onPointerEnter(); + }); + expect(onPartitionsIntent).toHaveBeenCalledTimes(1); + + await act(() => { + outer.props.onFocusCapture(); + }); + expect(onPartitionsIntent).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/web/src/routes/_chat.pull-requests.tsx b/apps/web/src/routes/_chat.pull-requests.tsx index 6f1f5e400322..1ea092901e86 100644 --- a/apps/web/src/routes/_chat.pull-requests.tsx +++ b/apps/web/src/routes/_chat.pull-requests.tsx @@ -2205,7 +2205,7 @@ function ExpandableSearch({ * window drag region throughout; its interactive children opt out through the `.drag-region` * descendant rules. */ -function PullRequestsColumn({ +export function PullRequestsColumn({ refreshing, onRefresh, searchValue,