authorgravatar for adam.bogucki2018@gmail.comAdamulek123 <adam.bogucki2018@gmail.com> 2026-09-01 13:07:27+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-01 04:07:27-07:00
log3c73fa7ce02b7ee6b2904ec58e47c06262aebf5d
tree8df36971c48515b0e9f7780dbf99662a5f3c681d
parent62d39bf00d5ddd83a9b36a81321fe9aa4d2502bb
signature Signed by PGP key B5690EEEBB952194

perf(web): defer pull request line stats until visible (#6471)

Co-authored-by: Theo Browne <me@t3.gg>

5 files changed, 561 insertions(+), 61 deletions(-)

apps/web/src/components/pullRequest/PullRequestRow.tsx+8-1
...@@ -1,5 +1,5 @@...@@ -1,5 +1,5 @@
1import { SearchIcon } from "lucide-react";1import { SearchIcon } from "lucide-react";
2import { memo } from "react";2import { memo, type RefCallback } from "react";
33
4import { cn } from "~/lib/utils";4import { cn } from "~/lib/utils";
5import { getSourceControlPresentationForKind } from "~/sourceControlPresentation";5import { getSourceControlPresentationForKind } from "~/sourceControlPresentation";
...@@ -40,6 +40,8 @@ function PullRequestRowImpl({...@@ -40,6 +40,8 @@ function PullRequestRowImpl({
40 showProvider,40 showProvider,
41 environmentLabel,41 environmentLabel,
42 matchedElsewhere,42 matchedElsewhere,
43 statsKey,
44 statsRef,
43 onSelect,45 onSelect,
44}: {46}: {
45 entry: EnvironmentPullRequestEntry;47 entry: EnvironmentPullRequestEntry;
...@@ -54,11 +56,16 @@ function PullRequestRowImpl({...@@ -54,11 +56,16 @@ function PullRequestRowImpl({
54 * commit message. Saying so is the difference between a result and an apparently random row.56 * commit message. Saying so is the difference between a result and an apparently random row.
55 */57 */
56 matchedElsewhere?: boolean;58 matchedElsewhere?: boolean;
59 /** Used by the list's shared visibility observer to defer optional line-count reads. */
60 statsKey?: string;
61 statsRef?: RefCallback<HTMLButtonElement>;
57 onSelect: (entry: EnvironmentPullRequestEntry) => void;62 onSelect: (entry: EnvironmentPullRequestEntry) => void;
58}) {63}) {
59 const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider);64 const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider);
60 return (65 return (
61 <button66 <button
67 ref={statsRef}
68 data-pull-request-stats-key={statsKey}
62 type="button"69 type="button"
63 aria-current={selected ? "true" : undefined}70 aria-current={selected ? "true" : undefined}
64 onClick={() => onSelect(entry)}71 onClick={() => onSelect(entry)}
apps/web/src/components/pullRequest/pullRequestList.logic.test.ts+196
...@@ -11,6 +11,10 @@ import {...@@ -11,6 +11,10 @@ import {
11 matchesPullRequestFilters,11 matchesPullRequestFilters,
12 matchesPullRequestQuery,12 matchesPullRequestQuery,
13 parsePullRequestQuery,13 parsePullRequestQuery,
14 pullRequestStatsBatches,
15 pullRequestStatsKeysToRequest,
16 pullRequestStatsRefreshBatches,
17 pullRequestStatsRequestBatches,
14 mergePullRequestDiffStats,18 mergePullRequestDiffStats,
15 narrowPullRequestsToFilters,19 narrowPullRequestsToFilters,
16 partitionPullRequestsWithPriority,20 partitionPullRequestsWithPriority,
...@@ -18,6 +22,7 @@ import {...@@ -18,6 +22,7 @@ import {
18 writePullRequestListSnapshot,22 writePullRequestListSnapshot,
19 rankPullRequestMatches,23 rankPullRequestMatches,
20 scorePullRequestMatch,24 scorePullRequestMatch,
25 retainVisiblePullRequestStatsBatches,
21 withDiffStat,26 withDiffStat,
22 resolveProjectScope,27 resolveProjectScope,
23 resolveQueryEnvironmentIds,28 resolveQueryEnvironmentIds,
...@@ -56,6 +61,197 @@ function entry(...@@ -56,6 +61,197 @@ function entry(
56 } as EnvironmentPullRequestEntry;61 } as EnvironmentPullRequestEntry;
57}62}
5863
64describe("visible pull request line-count targets", () => {
65 it("does not request a row again after its received batch is pruned", () => {
66 const entries = [entry({ number: 1 }), entry({ number: 2 })];
67 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
68 const firstKey = pullRequestEntryKey(entries[0]!);
69 const secondKey = pullRequestEntryKey(entries[1]!);
70 const completedStats = mergePullRequestDiffStats(new Map(), [
71 {
72 environmentId: ENV_1,
73 projectId: "project-1",
74 number: 1,
75 additions: 1,
76 deletions: 1,
77 },
78 ]);
79
80 const keys = pullRequestStatsKeysToRequest(
81 entriesByKey,
82 new Set([firstKey, secondKey]),
83 [],
84 completedStats,
85 );
86 expect([...keys]).toEqual([secondKey]);
87 expect(pullRequestStatsBatches(entriesByKey, keys)[0]?.input.refs).toEqual([
88 { projectId: "project-1", repository: "pingdotgg/t3code", number: 2 },
89 ]);
90 });
91
92 it("drops historical rows after a long scroll so refresh stays bounded to the viewport", () => {
93 const entries = Array.from({ length: 500 }, (_, index) => entry({ number: index + 1 }));
94 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
95 const batches = entries.map(
96 (item) => pullRequestStatsBatches(entriesByKey, new Set([pullRequestEntryKey(item)]))[0]!,
97 );
98 const visibleKeys = new Set(entries.slice(-12).map(pullRequestEntryKey));
99
100 const retained = retainVisiblePullRequestStatsBatches(batches, visibleKeys);
101 expect(retained).toHaveLength(12);
102 expect(retained.flatMap((batch) => batch.input.refs.map((ref) => ref.number))).toEqual(
103 entries.slice(-12).map((item) => item.number),
104 );
105 });
106
107 it("keeps every per-environment batch within the stats contract limit", () => {
108 const entries = Array.from({ length: 501 }, (_, index) => entry({ number: index + 1 }));
109 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
110 const batches = pullRequestStatsBatches(
111 entriesByKey,
112 new Set(entries.map(pullRequestEntryKey)),
113 );
114
115 expect(batches.map((batch) => batch.input.refs.length)).toEqual([500, 1]);
116 expect(batches.flatMap((batch) => [...batch.keys])).toEqual(entries.map(pullRequestEntryKey));
117 });
118
119 it("selects visible rows for date modes and every uncached row for size modes", () => {
120 const entries = [
121 entry({ number: 1 }),
122 entry({ number: 2 }),
123 entry({ number: 3, environmentId: "env-2" as EnvironmentId }),
124 ];
125 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
126 const [firstKey, secondKey] = entries.map(pullRequestEntryKey);
127 const cachedStats = mergePullRequestDiffStats(new Map(), [
128 {
129 environmentId: ENV_1,
130 projectId: "project-1",
131 number: 1,
132 additions: 1,
133 deletions: 1,
134 },
135 ]);
136
137 const visible = pullRequestStatsRequestBatches({
138 entriesByKey,
139 candidateKeys: new Set([firstKey!, secondKey!]),
140 policy: "visible",
141 activeBatches: [],
142 statsByRow: cachedStats,
143 });
144 expect(visible.flatMap((batch) => batch.input.refs.map((ref) => ref.number))).toEqual([2]);
145
146 const eager = pullRequestStatsRequestBatches({
147 entriesByKey,
148 candidateKeys: new Set([firstKey!]),
149 policy: "eager",
150 activeBatches: [],
151 statsByRow: cachedStats,
152 });
153 expect(eager.map((batch) => batch.environmentId)).toEqual([ENV_1, "env-2"]);
154 expect(eager.flatMap((batch) => batch.input.refs.map((ref) => ref.number))).toEqual([2, 3]);
155 });
156
157 it("refreshes only visible rows unless size sorting needs every loaded row", () => {
158 const entries = [entry({ number: 1 }), entry({ number: 2 }), entry({ number: 3 })];
159 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
160 const visibleKeys = new Set([pullRequestEntryKey(entries[1]!)]);
161 const cachedStats = mergePullRequestDiffStats(
162 new Map(),
163 entries.map((item) => ({
164 environmentId: ENV_1,
165 projectId: item.projectId,
166 number: item.number,
167 additions: item.additions,
168 deletions: item.deletions,
169 })),
170 );
171
172 const visible = pullRequestStatsRequestBatches({
173 entriesByKey,
174 candidateKeys: visibleKeys,
175 policy: "visible",
176 activeBatches: [],
177 statsByRow: cachedStats,
178 refresh: true,
179 });
180 expect(visible[0]?.input.refs.map((ref) => ref.number)).toEqual([2]);
181
182 const eager = pullRequestStatsRequestBatches({
183 entriesByKey,
184 candidateKeys: visibleKeys,
185 policy: "eager",
186 activeBatches: [],
187 statsByRow: cachedStats,
188 refresh: true,
189 });
190 expect(eager[0]?.input.refs.map((ref) => ref.number)).toEqual([1, 2, 3]);
191 });
192
193 it("ignores a late refresh after the filter or stats policy changes", () => {
194 const entries = [entry({ number: 1 }), entry({ number: 2 })];
195 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
196 const visibleKeys = new Set([pullRequestEntryKey(entries[1]!)]);
197 const requestedScope = { key: "open", policy: "visible" } as const;
198 const refresh = (currentScope: { key: string; policy: "visible" | "eager" }) =>
199 pullRequestStatsRefreshBatches({
200 requestedScope,
201 currentScope,
202 entriesByKey,
203 candidateKeys: visibleKeys,
204 statsByRow: new Map(),
205 });
206
207 expect(refresh(requestedScope)?.[0]?.input.refs.map((ref) => ref.number)).toEqual([2]);
208 expect(refresh({ key: "closed", policy: "visible" })).toBeNull();
209 expect(refresh({ key: "open", policy: "eager" })).toBeNull();
210 });
211
212 it("does not add request batches again while rows are active or cached", () => {
213 const entries = [entry({ number: 1 }), entry({ number: 2 })];
214 const entriesByKey = new Map(entries.map((item) => [pullRequestEntryKey(item), item]));
215 const first = pullRequestStatsRequestBatches({
216 entriesByKey,
217 candidateKeys: new Set(),
218 policy: "eager",
219 activeBatches: [],
220 statsByRow: new Map(),
221 });
222
223 expect(
224 pullRequestStatsRequestBatches({
225 entriesByKey,
226 candidateKeys: new Set(),
227 policy: "eager",
228 activeBatches: first,
229 statsByRow: new Map(),
230 }),
231 ).toEqual([]);
232
233 const cachedStats = mergePullRequestDiffStats(
234 new Map(),
235 entries.map((item) => ({
236 environmentId: ENV_1,
237 projectId: item.projectId,
238 number: item.number,
239 additions: item.additions,
240 deletions: item.deletions,
241 })),
242 );
243 expect(
244 pullRequestStatsRequestBatches({
245 entriesByKey,
246 candidateKeys: new Set(entries.map(pullRequestEntryKey)),
247 policy: "visible",
248 activeBatches: [],
249 statsByRow: cachedStats,
250 }),
251 ).toEqual([]);
252 });
253});
254
59describe("pull request involvement filtering", () => {255describe("pull request involvement filtering", () => {
60 const entries = [256 const entries = [
61 entry({ number: 1, author: { login: "Bilal", name: null, avatarUrl: null } }),257 entry({ number: 1, author: { login: "Bilal", name: null, avatarUrl: null } }),
apps/web/src/components/pullRequest/pullRequestList.logic.ts+150
...@@ -8,6 +8,7 @@ import {...@@ -8,6 +8,7 @@ import {
8 resolvePullRequestAuthorFilter,8 resolvePullRequestAuthorFilter,
9} from "@t3tools/contracts";9} from "@t3tools/contracts";
10import type {10import type {
11 ProjectId,
11 PullRequestActor,12 PullRequestActor,
12 PullRequestDiffStat,13 PullRequestDiffStat,
13 PullRequestInvolvement,14 PullRequestInvolvement,
...@@ -429,6 +430,155 @@ export function pullRequestEntryKey(entry: ScopedEntry): string {...@@ -429,6 +430,155 @@ export function pullRequestEntryKey(entry: ScopedEntry): string {
429 return `${scope}${entry.host}:${entry.repository}#${entry.number}`;430 return `${scope}${entry.host}:${entry.repository}#${entry.number}`;
430}431}
431432
433export interface PullRequestStatsTarget {
434 readonly environmentId: EnvironmentId;
435 readonly input: {
436 readonly refs: ReadonlyArray<{
437 readonly projectId: ProjectId;
438 readonly repository: string;
439 readonly number: number;
440 }>;
441 };
442}
443
444export interface PullRequestStatsBatch extends PullRequestStatsTarget {
445 readonly keys: ReadonlySet<string>;
446}
447
448export type PullRequestStatsPolicy = "visible" | "eager";
449
450export interface PullRequestStatsScope {
451 readonly key: string;
452 readonly policy: PullRequestStatsPolicy;
453}
454
455const MAX_PULL_REQUEST_STATS_REFS = 500;
456
457/** Excludes rows already covered by an active batch or the received-count cache. */
458export function pullRequestStatsKeysToRequest(
459 entriesByKey: ReadonlyMap<string, EnvironmentPullRequestEntry>,
460 enteredKeys: ReadonlySet<string>,
461 batches: ReadonlyArray<PullRequestStatsBatch>,
462 statsByRow: ReadonlyMap<string, unknown>,
463): ReadonlySet<string> {
464 const requested = new Set(batches.flatMap((batch) => [...batch.keys]));
465 return new Set(
466 [...enteredKeys].filter((key) => {
467 const entry = entriesByKey.get(key);
468 return (
469 entry !== undefined && !requested.has(key) && !statsByRow.has(pullRequestDiffStatKey(entry))
470 );
471 }),
472 );
473}
474
475/** Groups selected rows into bounded, immutable line-count reads per environment. */
476export function pullRequestStatsBatches(
477 entriesByKey: ReadonlyMap<string, EnvironmentPullRequestEntry>,
478 keys: ReadonlySet<string>,
479): ReadonlyArray<PullRequestStatsBatch> {
480 const byEnvironment = new Map<
481 EnvironmentId,
482 Array<{
483 readonly key: string;
484 readonly ref: PullRequestStatsTarget["input"]["refs"][number];
485 }>
486 >();
487 for (const key of keys) {
488 const entry = entriesByKey.get(key);
489 if (entry === undefined) continue;
490 const rows = byEnvironment.get(entry.environmentId) ?? [];
491 rows.push({
492 key,
493 ref: {
494 projectId: entry.projectId,
495 repository: entry.repository,
496 number: entry.number,
497 },
498 });
499 byEnvironment.set(entry.environmentId, rows);
500 }
501 return [...byEnvironment].flatMap(([environmentId, rows]) => {
502 const batches: PullRequestStatsBatch[] = [];
503 for (let index = 0; index < rows.length; index += MAX_PULL_REQUEST_STATS_REFS) {
504 const batch = rows.slice(index, index + MAX_PULL_REQUEST_STATS_REFS);
505 batches.push({
506 environmentId,
507 input: { refs: batch.map((row) => row.ref) },
508 keys: new Set(batch.map((row) => row.key)),
509 });
510 }
511 return batches;
512 });
513}
514
515/**
516 * Selects the next immutable stats batches. Size sorting needs every loaded row, while the other
517 * modes only need rows near the viewport. Normal reads skip active and cached rows; an explicit
518 * refresh asks for the selected rows again.
519 */
520export function pullRequestStatsRequestBatches({
521 entriesByKey,
522 candidateKeys,
523 policy,
524 activeBatches,
525 statsByRow,
526 refresh = false,
527}: {
528 readonly entriesByKey: ReadonlyMap<string, EnvironmentPullRequestEntry>;
529 readonly candidateKeys: ReadonlySet<string>;
530 readonly policy: PullRequestStatsPolicy;
531 readonly activeBatches: ReadonlyArray<PullRequestStatsBatch>;
532 readonly statsByRow: ReadonlyMap<string, unknown>;
533 readonly refresh?: boolean;
534}): ReadonlyArray<PullRequestStatsBatch> {
535 const requestedKeys = policy === "eager" ? new Set(entriesByKey.keys()) : candidateKeys;
536 const keys = refresh
537 ? requestedKeys
538 : pullRequestStatsKeysToRequest(entriesByKey, requestedKeys, activeBatches, statsByRow);
539 return pullRequestStatsBatches(entriesByKey, keys);
540}
541
542/** Ignores a refresh that finished after the list moved to another filter or stats policy. */
543export function pullRequestStatsRefreshBatches({
544 requestedScope,
545 currentScope,
546 entriesByKey,
547 candidateKeys,
548 statsByRow,
549}: {
550 readonly requestedScope: PullRequestStatsScope;
551 readonly currentScope: PullRequestStatsScope;
552 readonly entriesByKey: ReadonlyMap<string, EnvironmentPullRequestEntry>;
553 readonly candidateKeys: ReadonlySet<string>;
554 readonly statsByRow: ReadonlyMap<string, unknown>;
555}): ReadonlyArray<PullRequestStatsBatch> | null {
556 if (requestedScope.key !== currentScope.key || requestedScope.policy !== currentScope.policy) {
557 return null;
558 }
559 return pullRequestStatsRequestBatches({
560 entriesByKey,
561 candidateKeys,
562 policy: requestedScope.policy,
563 activeBatches: [],
564 statsByRow,
565 refresh: true,
566 });
567}
568
569/** Drops completed batches once every row in them has left the observer window. */
570export function retainVisiblePullRequestStatsBatches(
571 batches: ReadonlyArray<PullRequestStatsBatch>,
572 visibleKeys: ReadonlySet<string>,
573): ReadonlyArray<PullRequestStatsBatch> {
574 return batches.filter((batch) => {
575 for (const key of batch.keys) {
576 if (visibleKeys.has(key)) return true;
577 }
578 return false;
579 });
580}
581
432/**582/**
433 * The priority groups built from the hosts' own answers rather than re-partitioned from the583 * The priority groups built from the hosts' own answers rather than re-partitioned from the
434 * paginated feed. The feed is sliced by recency, so an older authored or review-requested row584 * paginated feed. The feed is sliced by recency, so an older authored or review-requested row
apps/web/src/routes/_chat.pull-requests.tsx+191-52
...@@ -32,7 +32,15 @@ import {...@@ -32,7 +32,15 @@ import {
32 RefreshCwIcon,32 RefreshCwIcon,
33 SearchIcon,33 SearchIcon,
34} from "lucide-react";34} from "lucide-react";
35import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from "react";35import {
36 useCallback,
37 useEffect,
38 useMemo,
39 useRef,
40 useState,
41 type ReactNode,
42 type RefObject,
43} from "react";
3644
37import {45import {
38 filterPullRequestsByInvolvement,46 filterPullRequestsByInvolvement,
...@@ -57,9 +65,15 @@ import {...@@ -57,9 +65,15 @@ import {
57 withDiffStat,65 withDiffStat,
58 writePullRequestListSnapshot,66 writePullRequestListSnapshot,
59 scorePullRequestMatch,67 scorePullRequestMatch,
68 pullRequestStatsRefreshBatches,
69 pullRequestStatsRequestBatches,
70 retainVisiblePullRequestStatsBatches,
60 type EnvironmentPullRequestEntry,71 type EnvironmentPullRequestEntry,
61 type MergedPullRequestList,72 type MergedPullRequestList,
62 type PullRequestDiffStats,73 type PullRequestDiffStats,
74 type PullRequestStatsBatch,
75 type PullRequestStatsPolicy,
76 type PullRequestStatsScope,
63 type PullRequestPartitionsSnapshot,77 type PullRequestPartitionsSnapshot,
64} from "../components/pullRequest/pullRequestList.logic";78} from "../components/pullRequest/pullRequestList.logic";
65import { assignProjectsToEnvironments } from "../components/pullRequest/pullRequestProjectAssignment.logic";79import { assignProjectsToEnvironments } from "../components/pullRequest/pullRequestProjectAssignment.logic";
...@@ -273,6 +287,8 @@ export const Route = createFileRoute("/_chat/pull-requests")({...@@ -273,6 +287,8 @@ export const Route = createFileRoute("/_chat/pull-requests")({
273function PullRequestsRouteView() {287function PullRequestsRouteView() {
274 const search = Route.useSearch();288 const search = Route.useSearch();
275 const sort = search.sort ?? "updated";289 const sort = search.sort ?? "updated";
290 const statsPolicy: PullRequestStatsPolicy =
291 sort === "largest" || sort === "smallest" ? "eager" : "visible";
276 const navigate = useNavigate({ from: Route.fullPath });292 const navigate = useNavigate({ from: Route.fullPath });
277 const { environments } = useEnvironments();293 const { environments } = useEnvironments();
278 // Every connected environment that has said it can list pull requests. Sorted, so the query294 // Every connected environment that has said it can list pull requests. Sorted, so the query
...@@ -607,6 +623,8 @@ function PullRequestsRouteView() {...@@ -607,6 +623,8 @@ function PullRequestsRouteView() {
607 // Page size is view state, not a URL concern: a shared link should open the first page.623 // Page size is view state, not a URL concern: a shared link should open the first page.
608 const scopeKey = `${environmentKey}:${assignmentKey}:${search.state}:${search.involvement}:${scopedProjectId ?? ""}:${search.host ?? ""}:${search.draft ?? ""}:${search.review ?? ""}:${search.checks ?? ""}:${search.author ?? ""}:${search.labels?.join("\u0000") ?? ""}`;624 const scopeKey = `${environmentKey}:${assignmentKey}:${search.state}:${search.involvement}:${scopedProjectId ?? ""}:${search.host ?? ""}:${search.draft ?? ""}:${search.review ?? ""}:${search.checks ?? ""}:${search.author ?? ""}:${search.labels?.join("\u0000") ?? ""}`;
609 const filterKey = `${scopeKey}:${sentQuery}`;625 const filterKey = `${scopeKey}:${sentQuery}`;
626 const statsScopeRef = useRef<PullRequestStatsScope>({ key: filterKey, policy: statsPolicy });
627 statsScopeRef.current = { key: filterKey, policy: statsPolicy };
610 // Where the next slice carries on from, per repository within each environment, as that628 // Where the next slice carries on from, per repository within each environment, as that
611 // environment handed it back. Sending it is what makes a second page cost a second page rather629 // environment handed it back. Sending it is what makes a second page cost a second page rather
612 // than the whole list again — and a repository it does not name has run out and is not read a630 // than the whole list again — and a repository it does not name has run out and is not read a
...@@ -780,6 +798,7 @@ function PullRequestsRouteView() {...@@ -780,6 +798,7 @@ function PullRequestsRouteView() {
780 // of its own work is a button that gets pressed again, and buys the whole cascade twice.798 // of its own work is a button that gets pressed again, and buys the whole cascade twice.
781 const [invalidating, setInvalidating] = useState(false);799 const [invalidating, setInvalidating] = useState(false);
782 const refreshFromHost = async () => {800 const refreshFromHost = async () => {
801 const requestedStatsScope = statsScopeRef.current;
783 setInvalidating(true);802 setInvalidating(true);
784 try {803 try {
785 // Every environment the page is reading, since what the reader pressed refresh for is the804 // Every environment the page is reading, since what the reader pressed refresh for is the
...@@ -795,7 +814,18 @@ function PullRequestsRouteView() {...@@ -795,7 +814,18 @@ function PullRequestsRouteView() {
795 facetQuery.refresh();814 facetQuery.refresh();
796 authoredQuery.refresh();815 authoredQuery.refresh();
797 reviewingQuery.refresh();816 reviewingQuery.refresh();
798 statsQuery.refresh();817 const visible = visibleStatsKeys.current;
818 const batches = pullRequestStatsRefreshBatches({
819 requestedScope: requestedStatsScope,
820 currentScope: statsScopeRef.current,
821 entriesByKey: entriesByStatsKey.current,
822 candidateKeys: visible.key === requestedStatsScope.key ? visible.values : new Set(),
823 statsByRow: statsByRowRef.current,
824 });
825 if (batches !== null) {
826 setStatsTargetState({ key: requestedStatsScope.key, batches });
827 statsQuery.refresh(batches.map(({ environmentId, input }) => ({ environmentId, input })));
828 }
799 setDetailRefreshToken((token) => token + 1);829 setDetailRefreshToken((token) => token + 1);
800 };830 };
801 const refreshing = invalidating || listQuery.isPending;831 const refreshing = invalidating || listQuery.isPending;
...@@ -1122,6 +1152,7 @@ function PullRequestsRouteView() {...@@ -1122,6 +1152,7 @@ function PullRequestsRouteView() {
1122 viewers,1152 viewers,
1123 ]);1153 ]);
11241154
1155 const scrollRef = useRef<HTMLDivElement>(null);
1125 const sentinelRef = useRef<HTMLDivElement>(null);1156 const sentinelRef = useRef<HTMLDivElement>(null);
1126 useEffect(() => {1157 useEffect(() => {
1127 const sentinel = sentinelRef.current;1158 const sentinel = sentinelRef.current;
...@@ -1156,7 +1187,7 @@ function PullRequestsRouteView() {...@@ -1156,7 +1187,7 @@ function PullRequestsRouteView() {
1156 }1187 }
1157 },1188 },
1158 // Start the next page slightly before the sentinel is on screen.1189 // Start the next page slightly before the sentinel is on screen.
1159 { rootMargin: "240px" },1190 { root: scrollRef.current, rootMargin: "240px" },
1160 );1191 );
1161 observer.observe(sentinel);1192 observer.observe(sentinel);
1162 return () => observer.disconnect();1193 return () => observer.disconnect();
...@@ -1220,35 +1251,136 @@ function PullRequestsRouteView() {...@@ -1220,35 +1251,136 @@ function PullRequestsRouteView() {
1220 viewers,1251 viewers,
1221 ]);1252 ]);
12221253
1223 // Keyed by every row being shown — the partitions can hold rows the feed has not paged to —1254 // Date sorts keep optional line-count reads near the viewport. Size sorts need every loaded
1224 // so scrolling further asks only about what is new. One read per environment, each asking only1255 // count before their order is final. Received counts stay cached across both policies.
1225 // about its own rows: a reference names a project, and a project belongs to one machine.1256 const entriesByStatsKey = useRef<ReadonlyMap<string, EnvironmentPullRequestEntry>>(new Map());
1226 const statsTargets = useMemo(() => {1257 entriesByStatsKey.current = new Map(
1227 const refsByEnvironment = new Map<1258 groups.flatMap((group) =>
1228 EnvironmentId,1259 group.entries.map((entry) => [pullRequestEntryKey(entry), entry] as const),
1229 Array<{ projectId: ProjectId; repository: string; number: number }>1260 ),
1230 >();1261 );
1231 for (const group of groups) {1262 const visibleStatsKeys = useRef({ key: filterKey, values: new Set<string>() });
1232 for (const entry of group.entries) {1263 const [statsByRow, setStatsByRow] = useState<PullRequestDiffStats>(() => new Map());
1233 const refs = refsByEnvironment.get(entry.environmentId) ?? [];1264 const statsByRowRef = useRef(statsByRow);
1234 refs.push({1265 statsByRowRef.current = statsByRow;
1235 projectId: entry.projectId,1266 const [statsTargetState, setStatsTargetState] = useState<{
1236 repository: entry.repository,1267 readonly key: string;
1237 number: entry.number,1268 readonly batches: ReadonlyArray<PullRequestStatsBatch>;
1238 });1269 }>({ key: filterKey, batches: [] });
1239 refsByEnvironment.set(entry.environmentId, refs);1270 const statsBatches = statsTargetState.key === filterKey ? statsTargetState.batches : [];
1271 const statsTargets = useMemo(
1272 () =>
1273 statsBatches.map(({ environmentId, input }) => ({
1274 environmentId,
1275 input,
1276 })),
1277 [statsBatches],
1278 );
1279 const statsObserver = useRef<IntersectionObserver | null>(null);
1280 const statsRows = useRef(new Set<HTMLButtonElement>());
1281 const statsPending = useRef(true);
1282 const statsPolicyRef = useRef(statsPolicy);
1283 statsPolicyRef.current = statsPolicy;
1284 const registerStatsRow = useCallback((node: HTMLButtonElement | null) => {
1285 if (node === null || typeof IntersectionObserver === "undefined") return;
1286 statsRows.current.add(node);
1287 statsObserver.current?.observe(node);
1288 return () => {
1289 statsRows.current.delete(node);
1290 statsObserver.current?.unobserve(node);
1291 const key = node.dataset.pullRequestStatsKey;
1292 const visible = visibleStatsKeys.current;
1293 if (
1294 statsPolicyRef.current !== "visible" ||
1295 key === undefined ||
1296 !visible.values.delete(key) ||
1297 statsPending.current
1298 ) {
1299 return;
1240 }1300 }
1241 }1301 setStatsTargetState((current) => {
1242 return [...refsByEnvironment].map(([environmentId, refs]) => ({1302 if (current.key !== visible.key) return current;
1243 environmentId,1303 const batches = retainVisiblePullRequestStatsBatches(current.batches, visible.values);
1244 input: { refs },1304 return batches.length === current.batches.length ? current : { key: current.key, batches };
1245 }));1305 });
1246 }, [groups]);1306 };
1307 }, []);
1308 useEffect(() => {
1309 if (statsPolicy !== "eager") return;
1310 setStatsTargetState((current) => {
1311 const batches = current.key === filterKey ? current.batches : [];
1312 const added = pullRequestStatsRequestBatches({
1313 entriesByKey: entriesByStatsKey.current,
1314 candidateKeys: visibleStatsKeys.current.values,
1315 policy: statsPolicy,
1316 activeBatches: batches,
1317 statsByRow: statsByRowRef.current,
1318 });
1319 if (added.length === 0 && current.key === filterKey) return current;
1320 return { key: filterKey, batches: [...batches, ...added] };
1321 });
1322 }, [filterKey, groups, statsPolicy]);
1323 useEffect(() => {
1324 if (statsPolicy !== "visible" || typeof IntersectionObserver === "undefined") return;
1325 visibleStatsKeys.current = { key: filterKey, values: new Set() };
1326 const observer = new IntersectionObserver(
1327 (observed) => {
1328 const visible = visibleStatsKeys.current;
1329 if (visible.key !== filterKey) return;
1330 let changed = false;
1331 const entered = new Set<string>();
1332 for (const item of observed) {
1333 const key = (item.target as HTMLElement).dataset.pullRequestStatsKey;
1334 if (key === undefined) continue;
1335 const wasVisible = visible.values.has(key);
1336 if (item.isIntersecting && entriesByStatsKey.current.has(key)) {
1337 visible.values.add(key);
1338 if (!wasVisible) entered.add(key);
1339 } else {
1340 visible.values.delete(key);
1341 }
1342 changed ||= wasVisible !== visible.values.has(key);
1343 }
1344 if (!changed) return;
1345 setStatsTargetState((current) => {
1346 const batches = current.key === filterKey ? current.batches : [];
1347 const retained = statsPending.current
1348 ? batches
1349 : retainVisiblePullRequestStatsBatches(batches, visible.values);
1350 const added = pullRequestStatsRequestBatches({
1351 entriesByKey: entriesByStatsKey.current,
1352 candidateKeys: entered,
1353 policy: statsPolicy,
1354 activeBatches: batches,
1355 statsByRow: statsByRowRef.current,
1356 });
1357 if (added.length === 0 && retained.length === batches.length) return current;
1358 return { key: filterKey, batches: [...retained, ...added] };
1359 });
1360 },
1361 { root: scrollRef.current, rootMargin: "480px" },
1362 );
1363 statsObserver.current = observer;
1364 for (const row of statsRows.current) observer.observe(row);
1365 return () => {
1366 observer.disconnect();
1367 if (statsObserver.current === observer) statsObserver.current = null;
1368 };
1369 }, [filterKey, statsPolicy]);
1247 const statsQuery = usePullRequestListStats(statsTargets);1370 const statsQuery = usePullRequestListStats(statsTargets);
1371 statsPending.current = statsQuery.isPending;
1372 useEffect(() => {
1373 if (statsPolicy !== "visible" || statsQuery.isPending) return;
1374 const visible = visibleStatsKeys.current;
1375 setStatsTargetState((current) => {
1376 if (current.key !== filterKey || visible.key !== filterKey) return current;
1377 const batches = retainVisiblePullRequestStatsBatches(current.batches, visible.values);
1378 return batches.length === current.batches.length ? current : { key: current.key, batches };
1379 });
1380 }, [filterKey, statsPolicy, statsQuery.isPending]);
1248 // Adding or removing one row keys a fresh stats query with nothing in it yet, so the counts1381 // Adding or removing one row keys a fresh stats query with nothing in it yet, so the counts
1249 // are merged into what is already held rather than rebuilt: every count on screen stays until1382 // are merged into what is already held rather than rebuilt: every count on screen stays until
1250 // its replacement arrives.1383 // its replacement arrives.
1251 const [statsByRow, setStatsByRow] = useState<PullRequestDiffStats>(() => new Map());
1252 useEffect(() => {1384 useEffect(() => {
1253 const stats = statsQuery.stats;1385 const stats = statsQuery.stats;
1254 if (stats === null) return;1386 if (stats === null) return;
...@@ -1486,30 +1618,35 @@ function PullRequestsRouteView() {...@@ -1486,30 +1618,35 @@ function PullRequestsRouteView() {
1486 {group.label}1618 {group.label}
1487 </h2>1619 </h2>
1488 ) : null}1620 ) : null}
1489 {group.entries.map((entry) => (1621 {group.entries.map((entry) => {
1490 <PullRequestRow1622 const entryKey = pullRequestEntryKey(entry);
1491 key={pullRequestEntryKey(entry)}1623 return (
1492 entry={entry}1624 <PullRequestRow
1493 showProjectTitle1625 key={entryKey}
1494 showProvider={showProvider}1626 statsKey={entryKey}
1495 {...(capableEnvironments.length > 1 &&1627 statsRef={registerStatsRow}
1496 environmentLabels.get(entry.environmentId) !== undefined1628 entry={entry}
1497 ? { environmentLabel: environmentLabels.get(entry.environmentId)! }1629 showProjectTitle
1498 : {})}1630 showProvider={showProvider}
1499 // Ten is the floor the ranking gives a row whose own fields say nothing1631 {...(capableEnvironments.length > 1 &&
1500 // about the search: the host matched something this row cannot show.1632 environmentLabels.get(entry.environmentId) !== undefined
1501 matchedElsewhere={1633 ? { environmentLabel: environmentLabels.get(entry.environmentId)! }
1502 typedParsed.text.length > 0 &&1634 : {})}
1503 scorePullRequestMatch(entry, typedParsed.text) <= MATCHED_ELSEWHERE_SCORE1635 // Ten is the floor the ranking gives a row whose own fields say nothing
1504 }1636 // about the search: the host matched something this row cannot show.
1505 selected={1637 matchedElsewhere={
1506 selected?.environmentId === entry.environmentId &&1638 typedParsed.text.length > 0 &&
1507 selected.repository === entry.repository &&1639 scorePullRequestMatch(entry, typedParsed.text) <= MATCHED_ELSEWHERE_SCORE
1508 selected.number === entry.number1640 }
1509 }1641 selected={
1510 onSelect={selectEntry}1642 selected?.environmentId === entry.environmentId &&
1511 />1643 selected.repository === entry.repository &&
1512 ))}1644 selected.number === entry.number
1645 }
1646 onSelect={selectEntry}
1647 />
1648 );
1649 })}
1513 </div>1650 </div>
1514 ))}1651 ))}
1515 </div>1652 </div>
...@@ -1648,6 +1785,7 @@ function PullRequestsRouteView() {...@@ -1648,6 +1785,7 @@ function PullRequestsRouteView() {
1648 pullRequestsSupported && !rightPanelState.isOpen ? openPanelControls : null,1785 pullRequestsSupported && !rightPanelState.isOpen ? openPanelControls : null,
1649 rightPanelOpen: rightPanelState.isOpen,1786 rightPanelOpen: rightPanelState.isOpen,
1650 listBody,1787 listBody,
1788 scrollRef,
1651 };1789 };
16521790
1653 const activateSurface = (surface: PullRequestSurface) => {1791 const activateSurface = (surface: PullRequestSurface) => {
...@@ -1932,6 +2070,7 @@ function PullRequestsColumn({...@@ -1932,6 +2070,7 @@ function PullRequestsColumn({
1932 titlebarControls,2070 titlebarControls,
1933 rightPanelOpen,2071 rightPanelOpen,
1934 listBody,2072 listBody,
2073 scrollRef,
1935}: {2074}: {
1936 refreshing: boolean;2075 refreshing: boolean;
1937 onRefresh: () => void;2076 onRefresh: () => void;
...@@ -1950,8 +2089,8 @@ function PullRequestsColumn({...@@ -1950,8 +2089,8 @@ function PullRequestsColumn({
1950 titlebarControls: ReactNode;2089 titlebarControls: ReactNode;
1951 rightPanelOpen: boolean;2090 rightPanelOpen: boolean;
1952 listBody: ReactNode;2091 listBody: ReactNode;
2092 scrollRef: RefObject<HTMLDivElement | null>;
1953}) {2093}) {
1954 const scrollRef = useRef<HTMLDivElement | null>(null);
1955 const markerRef = useRef<HTMLDivElement | null>(null);2094 const markerRef = useRef<HTMLDivElement | null>(null);
1956 const [condensed, setCondensed] = useState(false);2095 const [condensed, setCondensed] = useState(false);
1957 useEffect(() => {2096 useEffect(() => {
apps/web/src/state/pullRequests.ts+16-8
...@@ -31,7 +31,7 @@ export interface EnvironmentQueryTarget<Input> {...@@ -31,7 +31,7 @@ export interface EnvironmentQueryTarget<Input> {
31}31}
3232
33interface MergedEnvironmentQueryView<A> {33interface MergedEnvironmentQueryView<A> {
34 /** One entry per environment that has answered, in the order the targets were given. */34 /** One entry per query target that has answered, in the order the targets were given. */
35 readonly values: ReadonlyArray<readonly [EnvironmentId, A]>;35 readonly values: ReadonlyArray<readonly [EnvironmentId, A]>;
36 /** The first environment that failed. Others may still have answered — this is not fatal. */36 /** The first environment that failed. Others may still have answered — this is not fatal. */
37 readonly error: string | null;37 readonly error: string | null;
...@@ -78,11 +78,16 @@ function createMergedEnvironmentQuery<Input, A>(...@@ -78,11 +78,16 @@ function createMergedEnvironmentQuery<Input, A>(
78 return function useMergedQuery(targets: ReadonlyArray<EnvironmentQueryTarget<Input>>) {78 return function useMergedQuery(targets: ReadonlyArray<EnvironmentQueryTarget<Input>>) {
79 const key = JSON.stringify(targets);79 const key = JSON.stringify(targets);
80 const view = useAtomValue(targets.length === 0 ? empty : family(key));80 const view = useAtomValue(targets.length === 0 ? empty : family(key));
81 const refresh = useCallback(() => {81 const refresh = useCallback(
82 for (const target of JSON.parse(key) as ReadonlyArray<EnvironmentQueryTarget<Input>>) {82 (override?: ReadonlyArray<EnvironmentQueryTarget<Input>>) => {
83 appAtomRegistry.refresh(atomFor(target));83 const refreshTargets =
84 }84 override ?? (JSON.parse(key) as ReadonlyArray<EnvironmentQueryTarget<Input>>);
85 }, [key]);85 for (const target of refreshTargets) {
86 appAtomRegistry.refresh(atomFor(target));
87 }
88 },
89 [key],
90 );
86 return { ...view, refresh };91 return { ...view, refresh };
87 };92 };
88}93}
...@@ -118,7 +123,10 @@ export function usePullRequestListStats(...@@ -118,7 +123,10 @@ export function usePullRequestListStats(
118 targets: ReadonlyArray<EnvironmentQueryTarget<PullRequestListStatsInput>>,123 targets: ReadonlyArray<EnvironmentQueryTarget<PullRequestListStatsInput>>,
119): {124): {
120 readonly stats: ReadonlyArray<EnvironmentPullRequestStat> | null;125 readonly stats: ReadonlyArray<EnvironmentPullRequestStat> | null;
121 readonly refresh: () => void;126 readonly isPending: boolean;
127 readonly refresh: (
128 targets?: ReadonlyArray<EnvironmentQueryTarget<PullRequestListStatsInput>>,
129 ) => void;
122} {130} {
123 const query = usePullRequestStatsQuery(targets);131 const query = usePullRequestStatsQuery(targets);
124 const stats = useMemo(132 const stats = useMemo(
...@@ -130,5 +138,5 @@ export function usePullRequestListStats(...@@ -130,5 +138,5 @@ export function usePullRequestListStats(
130 ),138 ),
131 [query.values],139 [query.values],
132 );140 );
133 return { stats, refresh: query.refresh };141 return { stats, isPending: query.isPending, refresh: query.refresh };
134}142}