Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,12 @@ export function PullRequestDetailPanel({
* An action changed this pull request on the host, so a list showing it is now out of date.
* Told rather than assumed: only the page knows whether it is showing one.
*/
onActed?: () => void;
/**
* Each host action as it goes: "sent" the moment it leaves, so a list can answer before the
* host does; "done" or "failed" when the host has spoken. Undefined for one the caller cannot
* name, which is only ever "done".
*/
onActed?: (action?: PullRequestAction, phase?: "sent" | "done" | "failed") => void;
/** Page-owned detail columns use this to clear the selected pull request. */
onClose?: () => void;
/**
Expand Down Expand Up @@ -961,6 +966,7 @@ export function PullRequestDetailPanel({
method?: PullRequestMergeMethod,
updateMethod?: PullRequestUpdateMethod,
) => {
onActed?.(action, "sent");
const result = await runAction({
environmentId,
input: {
Expand Down Expand Up @@ -988,6 +994,7 @@ export function PullRequestDetailPanel({
title: ACTION_FAILURE_LABELS[action],
description: readableFailure(failure, hint),
});
onActed?.(action, "failed");
return false;
}
toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] });
Expand All @@ -1001,7 +1008,7 @@ export function PullRequestDetailPanel({
} else {
refreshDetail();
}
onActed?.();
onActed?.(action, "done");
return true;
};

Expand Down
83 changes: 83 additions & 0 deletions apps/web/src/components/pullRequest/pullRequestList.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ import {
resolveQueryEnvironmentIds,
resolveSelectedEnvironmentId,
type EnvironmentPullRequestEntry,
applyPullRequestOverrides,
pullRequestOverrideAfterAction,
reusePullRequestEntries,
settlePullRequestOverrides,
} from "./pullRequestList.logic";
import {
pullRequestListPreferences,
Expand Down Expand Up @@ -1600,3 +1604,82 @@ describe("the priority groups against a paginated feed", () => {
).toEqual([6123]);
});
});

describe("pull request list overrides", () => {
const entry = (number: number, state: "open" | "closed" | "merged") =>
({
host: "github.com",
repository: "pingdotgg/t3code",
number,
state,
isDraft: false,
updatedAt: "2026-07-01T00:00:00Z",
labels: [],
}) as unknown as PullRequestListEntry;
const key = (row: { number: number }) => `#${row.number}`;

it("maps the actions that change a row's state and nothing else", () => {
const at = "2026-07-02T00:00:00Z";
expect(pullRequestOverrideAfterAction(entry(1, "open"), "close", at)).toEqual({
state: "closed",
updatedAt: at,
});
expect(pullRequestOverrideAfterAction(entry(1, "closed"), "reopen", at)?.state).toBe("open");
expect(pullRequestOverrideAfterAction(entry(1, "open"), "merge", at)?.state).toBe("merged");
expect(pullRequestOverrideAfterAction(entry(1, "open"), "draft", at)?.isDraft).toBe(true);
expect(pullRequestOverrideAfterAction(entry(1, "open"), "update-branch", at)).toBeNull();
});

it("writes the override over the row and drops it from a list whose state it left", () => {
const rows = [entry(1, "open"), entry(2, "open")];
const overrides = new Map([
["#1", { state: "closed" as const, updatedAt: "2026-07-03T00:00:00Z" }],
]);
expect(
applyPullRequestOverrides(rows, overrides, key, "open").map((row) => row.number),
).toEqual([2]);
const all = applyPullRequestOverrides(rows, overrides, key, "all");
expect(all.map((row) => [row.number, row.state])).toEqual([
[1, "closed"],
[2, "open"],
]);
expect(applyPullRequestOverrides(rows, new Map(), key, "open")).toBe(rows);
});

it("hands back the held object for a row a refresh did not change", () => {
const previous = [entry(1, "open"), entry(2, "open")];
const next = [{ ...entry(1, "open") }, { ...entry(2, "open"), state: "merged" as const }];
const reused = reusePullRequestEntries(previous, next, key);
expect(reused[0]).toBe(previous[0]);
expect(reused[1]).toBe(next[1]);
expect(
reusePullRequestEntries(previous, [{ ...entry(1, "open") }, { ...entry(2, "open") }], key),
).toBe(previous);
});
});

describe("pull request list override settlement", () => {
const entry = (number: number, state: "open" | "closed" | "merged") =>
({ number, state, isDraft: false, labels: [] }) as unknown as PullRequestListEntry;
const key = (row: { number: number }) => `#${row.number}`;

it("keeps an override until an answer agrees with it", () => {
const closed = { state: "closed" as const, updatedAt: "2026-07-03T00:00:00Z" };
const overrides = new Map([["#1", closed]]);
// A read from before the action still says open: the override stands.
expect(settlePullRequestOverrides(overrides, [entry(1, "open")], key, "open")).toBe(overrides);
// The row is gone from an open list: that is the host agreeing.
expect(settlePullRequestOverrides(overrides, [entry(2, "open")], key, "open").size).toBe(0);
// Absent from an "all" list says nothing, so the override stays.
expect(settlePullRequestOverrides(overrides, [entry(2, "open")], key, "all").size).toBe(1);
// Present as closed: confirmed.
expect(settlePullRequestOverrides(overrides, [entry(1, "closed")], key, "all").size).toBe(0);
});

it("does not hand back the old order when only the order changed", () => {
const previous = [entry(1, "open"), entry(2, "open")];
const swapped = reusePullRequestEntries(previous, [entry(2, "open"), entry(1, "open")], key);
expect(swapped).not.toBe(previous);
expect(swapped.map((row) => row.number)).toEqual([2, 1]);
});
});
111 changes: 111 additions & 0 deletions apps/web/src/components/pullRequest/pullRequestList.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import {
} from "@t3tools/contracts";
import type {
ProjectId,
PullRequestAction,
PullRequestActor,
PullRequestDiffStat,
PullRequestInvolvement,
PullRequestLabel,
PullRequestListCursors,
PullRequestListFilters,
PullRequestListState,
PullRequestState,
} from "@t3tools/contracts";

import { toSortableTimestamp } from "../../lib/threadSort";
Expand Down Expand Up @@ -1140,3 +1142,112 @@ export function withDiffStat<
const stat = statsByRow.get(pullRequestDiffStatKey(entry));
return stat === undefined ? entry : { ...entry, ...stat };
}

/**
* What a row should say the moment an action is sent, before any host has answered. The host
* is the record and a later read replaces this, but the reader pressed the button and should
* see the row answer at once: a closed pull request leaves an "open" list on the click, not
* after the reads that follow.
*/
export interface PullRequestListOverride {
readonly state: PullRequestState;
readonly isDraft?: boolean;
readonly updatedAt: string;
}

export function pullRequestOverrideAfterAction(
entry: Pick<PullRequestListEntry, "state" | "isDraft">,
action: PullRequestAction,
at: string,
): PullRequestListOverride | null {
switch (action) {
case "close":
return { state: "closed", updatedAt: at };
case "reopen":
return { state: "open", updatedAt: at };
case "merge":
return { state: "merged", updatedAt: at };
case "draft":
return { state: entry.state, isDraft: true, updatedAt: at };
case "ready":
return { state: entry.state, isDraft: false, updatedAt: at };
default:
return null;
}
}

/** The rows with their pending answers written over them, and the ones the list's state filter no longer holds dropped. */
export function applyPullRequestOverrides<Entry extends PullRequestListEntry>(
entries: ReadonlyArray<Entry>,
overrides: ReadonlyMap<string, PullRequestListOverride>,
keyOf: (entry: Entry) => string,
state: PullRequestListState,
): ReadonlyArray<Entry> {
if (overrides.size === 0) return entries;
const out: Entry[] = [];
for (const entry of entries) {
const override = overrides.get(keyOf(entry));
if (override === undefined) {
out.push(entry);
continue;
}
if (state !== "all" && override.state !== state) continue;
out.push({ ...entry, ...override });
}
return out;
}

/**
* A fresh answer with the rows it did not change handed back as the objects already held, so a
* memoized row whose data is the same does not render again. Every refresh otherwise rebuilds
* every entry, and a hundred rows repaint for the one that moved.
*/
export function reusePullRequestEntries<Entry extends PullRequestListEntry>(
previous: ReadonlyArray<Entry>,
next: ReadonlyArray<Entry>,
keyOf: (entry: Entry) => string,
): ReadonlyArray<Entry> {
if (previous.length === 0) return next;
const held = new Map(previous.map((entry) => [keyOf(entry), entry]));
let reused = 0;
const out = next.map((entry) => {
const before = held.get(keyOf(entry));
if (before !== undefined && JSON.stringify(before) === JSON.stringify(entry)) {
reused += 1;
return before;
}
return entry;
});
return reused === next.length &&
previous.length === next.length &&
previous.every((entry, index) => keyOf(entry) === keyOf(next[index]!))
? previous
: out;
}

/**
* The overrides a whole-page answer has confirmed, dropped; the rest kept. A read that started
* before the action can land after it and still say the old thing, so an override is not
* cleared because an answer arrived but because the answer agrees: the row is there in the
* state the override said, or it is gone from a list whose state filter no longer holds it.
*/
export function settlePullRequestOverrides<Entry extends PullRequestListEntry>(
overrides: ReadonlyMap<string, PullRequestListOverride>,
answered: ReadonlyArray<Entry>,
keyOf: (entry: Entry) => string,
state: PullRequestListState,
): ReadonlyMap<string, PullRequestListOverride> {
if (overrides.size === 0) return overrides;
const byKey = new Map(answered.map((entry) => [keyOf(entry), entry]));
const kept = new Map<string, PullRequestListOverride>();
for (const [key, override] of overrides) {
const row = byKey.get(key);
const confirmed =
row === undefined
? state !== "all" && override.state !== state
: row.state === override.state &&
(override.isDraft === undefined || row.isDraft === override.isDraft);
if (!confirmed) kept.set(key, override);
}
return kept.size === overrides.size ? overrides : kept;
}
84 changes: 78 additions & 6 deletions apps/web/src/routes/_chat.pull-requests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { pullRequestHostOf, resolveEnvironmentMachineKind } from "@t3tools/contr
import type {
EnvironmentId,
ProjectId,
PullRequestAction,
PullRequestInvolvement,
PullRequestListCursors,
PullRequestListFilters,
Expand Down Expand Up @@ -77,6 +78,11 @@ import {
type PullRequestStatsPolicy,
type PullRequestStatsScope,
type PullRequestPartitionsSnapshot,
applyPullRequestOverrides,
type PullRequestListOverride,
pullRequestOverrideAfterAction,
reusePullRequestEntries,
settlePullRequestOverrides,
} from "../components/pullRequest/pullRequestList.logic";
import {
pullRequestListPreferences,
Expand Down Expand Up @@ -914,6 +920,18 @@ function PullRequestsRouteView() {
key: string;
entries: ReadonlyArray<EnvironmentPullRequestEntry>;
} | null>(null);
// What the reader just did to a row, shown before any host confirms it. A closed pull request
// leaves an "open" list on the click; the reads that follow are slow, and until one lands the
// row says what was asked of it. Cleared when a whole-page answer arrives after the action.
const [overrides, setOverrides] = useState<ReadonlyMap<string, PullRequestListOverride>>(
() => new Map(),
);
const overrideEntry = (entry: EnvironmentPullRequestEntry, action: PullRequestAction) => {
const override = pullRequestOverrideAfterAction(entry, action, new Date().toISOString());
if (override === null) return false;
setOverrides((current) => new Map(current).set(pullRequestEntryKey(entry), override));
return true;
};
// A reload recreates the registry the queries live in, so with nothing held the page would
// cold-start into skeletons even though almost every row is unchanged. The last answer for
// this set of environments is kept across reloads and hydrated here as the carried rows: they
Expand Down Expand Up @@ -1077,9 +1095,22 @@ function PullRequestsRouteView() {
// since the last read at the bottom of the page — below rows a week older — where "the
// latest" is exactly what a refresh was for. The host answers in the order the page
// reads, so its order stands; a row that moved was updated, and moving is the news.
return { key: filterKey, entries: rankPullRequestMatches(answered.entries, sentParsed.text) };
return {
key: filterKey,
entries: reusePullRequestEntries(
previous.entries,
rankPullRequestMatches(answered.entries, sentParsed.text),
pullRequestEntryKey,
),
};
});
// The host's word outranks the reader's, once it has actually said it: an override is
// cleared by an answer that agrees with it, not by any answer that happens to land.
setOverrides((current) =>
settlePullRequestOverrides(current, answered.entries, pullRequestEntryKey, search.state),
);
}, [
search.state,
answered,
filterKey,
sentCursors,
Expand Down Expand Up @@ -1455,9 +1486,17 @@ function PullRequestsRouteView() {
setStatsByRow((previous) => mergePullRequestDiffStats(previous, stats));
}, [statsQuery.stats]);
const displayGroups = useMemo(() => {
// The reader's pending answers go on here, after grouping: the authored and reviewing
// groups are read separately from the feed, and a row closed a moment ago has to leave
// whichever group it was in.
const enriched = groups.map((group) => ({
...group,
entries: group.entries.map((entry) => withDiffStat(entry, statsByRow)),
entries: applyPullRequestOverrides(
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
Outdated
group.entries.map((entry) => withDiffStat(entry, statsByRow)),
overrides,
pullRequestEntryKey,
search.state,
),
Comment thread
maria-rcks marked this conversation as resolved.
Outdated
}));
// Searching keeps its relevance order and priority groups unless the reader explicitly asks
// for another sort. The readiness queue is the default browse order, not a way to bury a
Expand All @@ -1470,7 +1509,16 @@ function PullRequestsRouteView() {
entry.additions + entry.deletions > 0 || statsByRow.has(pullRequestDiffStatKey(entry)),
search.involvement,
);
}, [groups, search.involvement, sort, statsByRow, typedParsed.text]);
}, [groups, overrides, search.involvement, search.state, sort, statsByRow, typedParsed.text]);
const heldPullRequestsBySurface = useMemo(
() =>
new Map(
groups.flatMap((group) =>
group.entries.map((entry) => [pullRequestListEntryId(entry), entry] as const),
),
),
[groups],
);
const listedPullRequestsBySurface = useMemo(
() =>
new Map(
Expand Down Expand Up @@ -2067,9 +2115,33 @@ function PullRequestsRouteView() {
refreshToken={detailRefreshToken}
// Host actions can change both readiness and diff size, so refresh the counts
// alongside the list. The panel already refreshes itself after each action.
onActed={() => {
// Mutations already invalidate the host's affected caches.
refreshListAndStats(undefined, panelEnvironmentId);
onActed={(action, phase = "done") => {
// An action that only moves a row's state is written onto the row as it is
// sent, and taken back if the host refuses; the host invalidates its caches,
// so the next scheduled read confirms it. The rest change what the counts and
// checks say, and those need the reads once they are done.
// From every row held, not the ones on screen: a pull request closed from an
// open list has left the screen, and reopening it has to find it anyway.
const acted = heldPullRequestsBySurface.get(
pullRequestListEntryId(renderedPullRequestSurface),
);
const stateOnly =
action !== undefined &&
acted !== undefined &&
pullRequestOverrideAfterAction(acted, action, "") !== null;
Comment thread
maria-rcks marked this conversation as resolved.
Outdated
if (stateOnly) {
if (phase === "sent") overrideEntry(acted, action);
if (phase === "failed") {
Comment thread
maria-rcks marked this conversation as resolved.
Outdated
const key = pullRequestEntryKey(acted);
setOverrides((current) => {
const next = new Map(current);
next.delete(key);
return next;
});
}
return;
}
if (phase === "done") refreshListAndStats(undefined, panelEnvironmentId);
}}
/>
</RightPanelTabs>
Expand Down
Loading