authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-09-04 23:55:15-04:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-04 23:55:15-04:00
log931d41f933b2536f699975412169f5318e6adad5
treeb433f860576b001843904dfb341cfbbef19459f4
parent9cb40178a53cca279c67a9079afab3cddf6b6ddb
signature Signed by PGP key B5690EEEBB952194

fix(web): sort equally merge-ready pull requests by diff size (#9887)

3 files changed, 39 insertions(+), 18 deletions(-)

apps/web/src/components/pullRequest/pullRequestList.logic.test.ts+24-7
...@@ -723,6 +723,7 @@ describe("default merge-readiness ranking", () => {...@@ -723,6 +723,7 @@ describe("default merge-readiness ranking", () => {
723 number: 4,723 number: 4,
724 checksState: "passing",724 checksState: "passing",
725 reviewDecision: "approved",725 reviewDecision: "approved",
726 additions: 1_000,
726 updatedAt: "2026-06-01T00:00:00Z",727 updatedAt: "2026-06-01T00:00:00Z",
727 });728 });
728 const draft = entry({729 const draft = entry({
...@@ -747,7 +748,7 @@ describe("default merge-readiness ranking", () => {...@@ -747,7 +748,7 @@ describe("default merge-readiness ranking", () => {
747 ).toEqual([4, 3, 5, 2, 6, 1]);748 ).toEqual([4, 3, 5, 2, 6, 1]);
748 });749 });
749750
750 it("uses recency inside one readiness tier", () => {751 it("uses recency when readiness and diff size tie", () => {
751 const older = entry({ number: 1, checksState: "passing" });752 const older = entry({ number: 1, checksState: "passing" });
752 const newer = entry({753 const newer = entry({
753 number: 2,754 number: 2,
...@@ -760,13 +761,13 @@ describe("default merge-readiness ranking", () => {...@@ -760,13 +761,13 @@ describe("default merge-readiness ranking", () => {
760 ]);761 ]);
761 });762 });
762763
763 it("keeps readiness order stable when optional diff counts arrive", () => {764 it("ranks smaller measured diffs first within a readiness tier as counts arrive", () => {
764 const larger = entry({765 const larger = entry({
765 number: 1,766 number: 1,
766 checksState: "passing",767 checksState: "passing",
767 reviewDecision: "approved",768 reviewDecision: "approved",
768 additions: 40,769 additions: 1,
769 deletions: 10,770 deletions: 49,
770 updatedAt: "2026-09-01T00:00:00Z",771 updatedAt: "2026-09-01T00:00:00Z",
771 });772 });
772 const smaller = entry({773 const smaller = entry({
...@@ -788,14 +789,30 @@ describe("default merge-readiness ranking", () => {...@@ -788,14 +789,30 @@ describe("default merge-readiness ranking", () => {
788789
789 expect(790 expect(
790 rankPullRequestsByMergeReadiness([larger, unknown, smaller]).map((row) => row.number),791 rankPullRequestsByMergeReadiness([larger, unknown, smaller]).map((row) => row.number),
791 ).toEqual([3, 1, 2]);792 ).toEqual([2, 1, 3]);
792 expect(793 expect(
793 rankPullRequestsByMergeReadiness([794 rankPullRequestsByMergeReadiness([
794 larger,795 larger,
795 { ...unknown, additions: 500, deletions: 200 },796 { ...unknown, additions: 1, deletions: 0 },
796 smaller,797 smaller,
797 ]).map((row) => row.number),798 ]).map((row) => row.number),
798 ).toEqual([3, 1, 2]);799 ).toEqual([3, 2, 1]);
800 });
801
802 it("distinguishes measured empty diffs from missing counts when sorting groups", () => {
803 const unknown = entry({ number: 1, additions: 0, deletions: 0 });
804 const measured = entry({ number: 2 });
805 const empty = entry({ number: 3, additions: 0, deletions: 0 });
806 const groups = [
807 { key: "others", label: "Others", entries: [unknown, measured, empty] },
808 ] as const;
809
810 const sorted = sortPullRequestGroups(groups, "ready", "", (row) => row.number !== 1);
811
812 expect(sorted[0]!.entries.map((row) => row.number)).toEqual([3, 2, 1]);
813 expect(sortPullRequestGroups(groups, "ready", "sidebar", (row) => row.number !== 1)).toEqual(
814 groups,
815 );
799 });816 });
800817
801 it("keeps authored work first and ranks each group by readiness", () => {818 it("keeps authored work first and ranks each group by readiness", () => {
apps/web/src/components/pullRequest/pullRequestList.logic.ts+7-3
...@@ -1010,10 +1010,12 @@ export function rankPullRequestMatches<Entry extends PullRequestListEntry>(...@@ -1010,10 +1010,12 @@ export function rankPullRequestMatches<Entry extends PullRequestListEntry>(
1010 * verdict, then everything else still open. Drafts stay in that third tier because their author1010 * verdict, then everything else still open. Drafts stay in that third tier because their author
1011 * has not made them mergeable yet. Finished work follows open work when all states are visible. A1011 * has not made them mergeable yet. Finished work follows open work when all states are visible. A
1012 * known conflict is never ready, whatever its checks, review or state say, so it stays at the1012 * known conflict is never ready, whatever its checks, review or state say, so it stays at the
1013 * bottom. Recency breaks ties, so optional diff counts never move the queue beneath the reader.1013 * bottom. Within each tier, smaller measured diffs come first, then unknown sizes. Recency
1014 * breaks ties between equally sized diffs.
1014 */1015 */
1015export function rankPullRequestsByMergeReadiness<Entry extends PullRequestListEntry>(1016export function rankPullRequestsByMergeReadiness<Entry extends PullRequestListEntry>(
1016 entries: ReadonlyArray<Entry>,1017 entries: ReadonlyArray<Entry>,
1018 hasMeasuredSize: (entry: Entry) => boolean = (entry) => entry.additions + entry.deletions > 0,
1017): ReadonlyArray<Entry> {1019): ReadonlyArray<Entry> {
1018 const tier = (entry: Entry) => {1020 const tier = (entry: Entry) => {
1019 if (entry.mergeability === "conflicting") return 4;1021 if (entry.mergeability === "conflicting") return 4;
...@@ -1026,7 +1028,9 @@ export function rankPullRequestsByMergeReadiness<Entry extends PullRequestListEn...@@ -1026,7 +1028,9 @@ export function rankPullRequestsByMergeReadiness<Entry extends PullRequestListEn
1026 return entries.toSorted((left, right) => {1028 return entries.toSorted((left, right) => {
1027 const byTier = tier(left) - tier(right);1029 const byTier = tier(left) - tier(right);
1028 if (byTier !== 0) return byTier;1030 if (byTier !== 0) return byTier;
1029 return right.updatedAt.localeCompare(left.updatedAt);1031 const measured = Number(hasMeasuredSize(right)) - Number(hasMeasuredSize(left));
1032 const sized = left.additions + left.deletions - (right.additions + right.deletions);
1033 return measured || sized || right.updatedAt.localeCompare(left.updatedAt);
1030 });1034 });
1031}1035}
10321036
...@@ -1042,7 +1046,7 @@ export function sortPullRequestGroups<Entry extends PullRequestListEntry>(...@@ -1042,7 +1046,7 @@ export function sortPullRequestGroups<Entry extends PullRequestListEntry>(
10421046
1043 if (sort === "ready") {1047 if (sort === "ready") {
1044 return searchText.trim().length === 01048 return searchText.trim().length === 0
1045 ? sortWithinGroups(rankPullRequestsByMergeReadiness)1049 ? sortWithinGroups((entries) => rankPullRequestsByMergeReadiness(entries, hasMeasuredSize))
1046 : groups;1050 : groups;
1047 }1051 }
1048 if (sort === "updated") return groups;1052 if (sort === "updated") return groups;
apps/web/src/routes/_chat.pull-requests.tsx+8-8
...@@ -292,7 +292,7 @@ function PullRequestsRouteView() {...@@ -292,7 +292,7 @@ function PullRequestsRouteView() {
292 const search = Route.useSearch();292 const search = Route.useSearch();
293 const sort = search.sort ?? "ready";293 const sort = search.sort ?? "ready";
294 const statsPolicy: PullRequestStatsPolicy =294 const statsPolicy: PullRequestStatsPolicy =
295 sort === "largest" || sort === "smallest" ? "eager" : "visible";295 sort === "ready" || sort === "largest" || sort === "smallest" ? "eager" : "visible";
296 const navigate = useNavigate({ from: Route.fullPath });296 const navigate = useNavigate({ from: Route.fullPath });
297 const keybindings = useAtomValue(primaryServerKeybindingsAtom);297 const keybindings = useAtomValue(primaryServerKeybindingsAtom);
298 const { environments } = useEnvironments();298 const { environments } = useEnvironments();
...@@ -826,7 +826,7 @@ function PullRequestsRouteView() {...@@ -826,7 +826,7 @@ function PullRequestsRouteView() {
826 // from the first moment rather than the second: a button that stays live through the slow half826 // from the first moment rather than the second: a button that stays live through the slow half
827 // of its own work is a button that gets pressed again, and buys the whole cascade twice.827 // of its own work is a button that gets pressed again, and buys the whole cascade twice.
828 const [invalidating, setInvalidating] = useState(false);828 const [invalidating, setInvalidating] = useState(false);
829 const refreshFromHost = async () => {829 const refreshFromHost = async (includeDetail = true) => {
830 const requestedStatsScope = statsScopeRef.current;830 const requestedStatsScope = statsScopeRef.current;
831 setInvalidating(true);831 setInvalidating(true);
832 try {832 try {
...@@ -851,7 +851,7 @@ function PullRequestsRouteView() {...@@ -851,7 +851,7 @@ function PullRequestsRouteView() {
851 setStatsTargetState({ key: requestedStatsScope.key, batches });851 setStatsTargetState({ key: requestedStatsScope.key, batches });
852 statsQuery.refresh(batches.map(({ environmentId, input }) => ({ environmentId, input })));852 statsQuery.refresh(batches.map(({ environmentId, input }) => ({ environmentId, input })));
853 }853 }
854 setDetailRefreshToken((token) => token + 1);854 if (includeDetail) setDetailRefreshToken((token) => token + 1);
855 };855 };
856 const refreshing = invalidating || listQuery.isPending;856 const refreshing = invalidating || listQuery.isPending;
857857
...@@ -1270,8 +1270,8 @@ function PullRequestsRouteView() {...@@ -1270,8 +1270,8 @@ function PullRequestsRouteView() {
1270 viewers,1270 viewers,
1271 ]);1271 ]);
12721272
1273 // Date sorts keep optional line-count reads near the viewport. Size sorts need every loaded1273 // Date sorts keep optional line-count reads near the viewport. Size and readiness sorts need
1274 // count before their order is final. Received counts stay cached across both policies.1274 // every loaded count before their order is final. Counts stay cached across both policies.
1275 const entriesByStatsKey = useRef<ReadonlyMap<string, EnvironmentPullRequestEntry>>(new Map());1275 const entriesByStatsKey = useRef<ReadonlyMap<string, EnvironmentPullRequestEntry>>(new Map());
1276 entriesByStatsKey.current = new Map(1276 entriesByStatsKey.current = new Map(
1277 groups.flatMap((group) =>1277 groups.flatMap((group) =>
...@@ -1950,10 +1950,10 @@ function PullRequestsRouteView() {...@@ -1950,10 +1950,10 @@ function PullRequestsRouteView() {
1950 ) ?? null1950 ) ?? null
1951 }1951 }
1952 refreshToken={detailRefreshToken}1952 refreshToken={detailRefreshToken}
1953 // Merging, closing or reopening changes the row this panel was opened from, so1953 // Host actions can change both readiness and diff size, so refresh the counts
1954 // the list behind it is out of date the moment the host takes the action.1954 // alongside the list. The panel already refreshes itself after each action.
1955 onActed={() => {1955 onActed={() => {
1956 refreshList(true);1956 void refreshFromHost(false);
1957 }}1957 }}
1958 />1958 />
1959 </RightPanelTabs>1959 </RightPanelTabs>