fix(board): spend the search limit per relation, batch the checks

server/board/buckets.test.ts

@@ -68,7 +68,7 @@ describe("mergeBucketResults", () => {
     expect(merged?.relations).toEqual(["author", "owned"]);
   });
 
-  it("applies the limit to the merged, sorted union rather than per bucket", () => {
+  it("applies the limit per relation and returns the union newest first", () => {
     const buckets: BucketResult[] = [
       {
         relation: "author",
@@ -86,9 +86,24 @@ describe("mergeBucketResults", () => {
       },
     ];
     const result = mergeBucketResults(buckets, toBoardItem, 2);
-    expect(result).toHaveLength(2);
-    // The two newest across both buckets, not the first two encountered.
-    expect(result.map((item) => item.id)).toEqual(["new-1", "new-2"]);
+    expect(result.map((item) => item.id)).toEqual(["new-1", "new-2", "old-2", "old-1"]);
+  });
+
+  it("keeps an older authored item when a newer relation fills the budget", () => {
+    // The review queue this models is what a shared budget spent entirely:
+    // every review request is newer than the one pull request the viewer wrote.
+    const buckets: BucketResult[] = [
+      {
+        relation: "review-requested",
+        nodes: [
+          { id: "review-1", fakeUpdatedAt: "2024-06-03T00:00:00Z" },
+          { id: "review-2", fakeUpdatedAt: "2024-06-02T00:00:00Z" },
+        ],
+      },
+      { relation: "author", nodes: [{ id: "mine", fakeUpdatedAt: "2024-01-01T00:00:00Z" }] },
+    ];
+    const result = mergeBucketResults(buckets, toBoardItem, 2);
+    expect(result.map((item) => item.id)).toContain("mine");
   });
 
   it("drops a node that toBoardItem rejects", () => {

server/board/buckets.ts

@@ -82,6 +82,12 @@ export async function runBuckets(
  * null for a node the caller wants dropped entirely (an archived discussion,
  * an empty node from the other inline fragment matching nothing), which is
  * why it runs before the relation is ever recorded.
+ *
+ * `limit` is a budget per relation, not one shared by the union. A shared
+ * budget is spent by whichever relation happens to have the most recently
+ * updated items: a full review queue is newer than almost anything else, so
+ * it took the whole list and left the viewer's own pull requests off a board
+ * that exists to show them.
  */
 export function mergeBucketResults<TNode extends GhSearchNode>(
   buckets: readonly BucketResult[],
@@ -110,5 +116,13 @@ export function mergeBucketResults<TNode extends GhSearchNode>(
   for (const item of byId.values()) {
     item.relations.sort((a, b) => RELATION_ORDER[a] - RELATION_ORDER[b]);
   }
-  return [...byId.values()].sort((a, b) => b.updatedAt.localeCompare(a.updatedAt)).slice(0, limit);
+
+  const items = [...byId.values()].sort((a, b) => b.updatedAt.localeCompare(a.updatedAt));
+  const kept = new Set<string>();
+  for (const relation of new Set(buckets.map((bucket) => bucket.relation))) {
+    for (const item of items.filter((item) => item.relations.includes(relation)).slice(0, limit)) {
+      kept.add(item.id);
+    }
+  }
+  return items.filter((item) => kept.has(item.id));
 }

server/board/checks.ts

@@ -11,9 +11,8 @@ import { gh } from "../github/gh";
  * would turn that into a blank Draft PRs *and* Open PRs column; asking for it
  * separately costs pills nobody could have seen anyway.
  *
- * `nodes(ids:)` takes at most 100 ids, which the caller cannot exceed: it asks
- * only for the open pull requests, and the merged list was already cut to
- * `limit`, whose own ceiling is 100.
+ * `nodes(ids:)` takes at most 100 ids, so `attachChecks` asks in batches of
+ * that size rather than assuming the open pull requests fit in one.
  */
 const CHECKS_QUERY = `query($ids: [ID!]!) {
   nodes(ids: $ids) {
@@ -57,6 +56,9 @@ const CHECKS_QUERY = `query($ids: [ID!]!) {
  */
 type CheckOutcome = "passed" | "failed" | "pending" | "ignored";
 
+/** GitHub's own ceiling on `nodes(ids:)`. */
+const CHECKS_BATCH = 100;
+
 /**
  * Mirrors Paseo's `mapCheckRunStatus` so the board and the sidebar cannot
  * disagree about the same pull request, with one deliberate difference:
@@ -211,16 +213,20 @@ async function fetchChecks(ids: readonly string[]): Promise<Map<string, CheckSum
 export async function attachChecks(items: readonly BoardItem[]): Promise<BoardItem[]> {
   const ids = items.map((item) => item.id).filter((id) => id !== "");
   if (ids.length === 0) return [...items];
-  let summaries: Map<string, CheckSummary>;
-  try {
-    summaries = await fetchChecks(ids);
-  } catch (error) {
-    console.warn(
-      `[github-board] pull request checks unavailable: ${
-        error instanceof Error ? error.message : String(error)
-      }`,
-    );
-    return [...items];
+
+  const summaries = new Map<string, CheckSummary>();
+  for (let start = 0; start < ids.length; start += CHECKS_BATCH) {
+    const batch = ids.slice(start, start + CHECKS_BATCH);
+    try {
+      for (const [id, summary] of await fetchChecks(batch)) summaries.set(id, summary);
+    } catch (error) {
+      // One batch failing costs its own pills, not every other batch's.
+      console.warn(
+        `[github-board] pull request checks unavailable: ${
+          error instanceof Error ? error.message : String(error)
+        }`,
+      );
+    }
   }
   return items.map((item) => ({ ...item, checks: summaries.get(item.id) ?? null }));
 }