fix(webapp): stop the Batches list hiding or mis-ordering batches for some orgs

d-cs · d-cs · commit 6d54da150091 · 2026-07-17T12:38:07.000+01:00
The dashboard Batches list ordered and paginated by batch id, but batch id no longer reflects
creation order for every batch, so some organizations could see older batches missing from the
list or sorted above newer ones. The list now orders and paginates by creation time (with id as a
stable tiebreak) and always reads both backing stores, so every batch appears exactly once,
newest first.
diff --git a/apps/webapp/app/presenters/v3/BatchListPresenter.server.ts b/apps/webapp/app/presenters/v3/BatchListPresenter.server.ts
@@ -43,6 +43,24 @@ type BatchRow = {
   batchVersion: string;
 };
 
+// Composite keyset cursor "<createdAt-epoch-ms>_<id>". Ordering is by createdAt then id: a batch id is
+// a cuid (legacy) OR a run-ops id (new), and the two schemes occupy different lexical ranges, so `id`
+// alone is not a valid chronological order across the residency split. `id` is the stable tiebreak.
+// Old plain-id cursors (no "_") decode to undefined and restart from page 1 (self-healing).
+type BatchCursor = { createdAt: Date; id: string };
+function encodeBatchCursor(row: BatchCursor): string {
+  return `${row.createdAt.getTime()}_${row.id}`;
+}
+function decodeBatchCursor(cursor: string | undefined): BatchCursor | undefined {
+  if (!cursor) return undefined;
+  const sep = cursor.indexOf("_");
+  if (sep === -1) return undefined;
+  const ms = Number(cursor.slice(0, sep));
+  const id = cursor.slice(sep + 1);
+  if (!Number.isFinite(ms) || id.length === 0) return undefined;
+  return { createdAt: new Date(ms), id };
+}
+
 export class BatchListPresenter extends BasePresenter {
   // Optional run-ops read-routing. Omitted (single-DB / self-host) => everything
   // reads from `_replica` exactly as today (passthrough). Field names are local to
@@ -86,17 +104,16 @@ export class BatchListPresenter extends BasePresenter {
       return scan(passthrough);
     }
 
-    const newRows = await scan(this.readRoute.runOpsNew ?? passthrough);
-
-    // New DB filled the page — skip the legacy read entirely; older rows fall on a later page.
-    if (newRows.length >= pageSize + 1) {
-      return newRows;
-    }
-
-    const legacyRows = await scan(this.readRoute.runOpsLegacyReplica ?? passthrough);
+    // Always read BOTH stores and merge. The old "skip legacy when new fills the page" shortcut is
+    // unsound across the residency split: legacy cuid ids ("c…") sort ABOVE new run-ops ids ("0…")
+    // under id order, so a new-only page can hide pre-flip legacy batches that belong ahead of it.
+    // Ordering is by createdAt (id tiebreak), which is chronologically correct across both schemes.
+    const [newRows, legacyRows] = await Promise.all([
+      scan(this.readRoute.runOpsNew ?? passthrough),
+      scan(this.readRoute.runOpsLegacyReplica ?? passthrough),
+    ]);
 
-    // De-dupe by id (new wins), re-sort under the page's keyset order, re-apply the over-fetch
-    // LIMIT — reproduces the pageSize+1 window a single union scan would return.
+    // De-dupe by id (new wins), re-sort under the page's keyset order, re-apply the over-fetch LIMIT.
     const byId = new Map<string, BatchRow>();
     for (const row of newRows) {
       byId.set(row.id, row);
@@ -107,10 +124,16 @@ export class BatchListPresenter extends BasePresenter {
       }
     }
 
-    // codepoint comparator (NEVER localeCompare): BatchTaskRun.id is ASCII (cuid or run-ops id).
-    const sign = direction === "forward" ? 1 : -1; // forward => DESC; backward => ASC
+    // forward => newest-first (createdAt DESC), backward => oldest-first (ASC); id is the stable
+    // tiebreak (ASCII codepoint, NEVER localeCompare).
+    const sign = direction === "forward" ? 1 : -1;
     return Array.from(byId.values())
-      .sort((a, b) => (a.id < b.id ? sign : a.id > b.id ? -sign : 0))
+      .sort((a, b) => {
+        const at = a.createdAt.getTime();
+        const bt = b.createdAt.getTime();
+        if (at !== bt) return at < bt ? sign : -sign;
+        return a.id < b.id ? sign : a.id > b.id ? -sign : 0;
+      })
       .slice(0, pageSize + 1);
   }
 
@@ -212,11 +235,28 @@ export class BatchListPresenter extends BasePresenter {
     }
     const createdAtLte: Date | undefined = time.to;
 
+    // Composite (createdAt, id) keyset — see encodeBatchCursor. An old plain-id cursor decodes to
+    // undefined and restarts from page 1.
+    const keyCursor = decodeBatchCursor(cursor);
+
     const batches = await this.#scanBatchTaskRun(pageSize, direction, (client) =>
       client.batchTaskRun.findMany({
         where: {
           runtimeEnvironmentId: environmentId,
-          ...(cursor ? { id: direction === "forward" ? { lt: cursor } : { gt: cursor } } : {}),
+          ...(keyCursor
+            ? {
+                OR:
+                  direction === "forward"
+                    ? [
+                        { createdAt: { lt: keyCursor.createdAt } },
+                        { createdAt: keyCursor.createdAt, id: { lt: keyCursor.id } },
+                      ]
+                    : [
+                        { createdAt: { gt: keyCursor.createdAt } },
+                        { createdAt: keyCursor.createdAt, id: { gt: keyCursor.id } },
+                      ],
+              }
+            : {}),
           ...(friendlyId ? { friendlyId } : {}),
           ...(statuses && statuses.length > 0
             ? { status: { in: statuses }, batchVersion: { not: "v1" } }
@@ -230,7 +270,10 @@ export class BatchListPresenter extends BasePresenter {
               }
             : {}),
         },
-        orderBy: { id: direction === "forward" ? "desc" : "asc" },
+        orderBy: [
+          { createdAt: direction === "forward" ? "desc" : "asc" },
+          { id: direction === "forward" ? "desc" : "asc" },
+        ],
         take: pageSize + 1,
         select: {
           id: true,
@@ -248,23 +291,24 @@ export class BatchListPresenter extends BasePresenter {
 
     const hasMore = batches.length > pageSize;
 
-    //get cursors for next and previous pages
+    //get cursors for next and previous pages (composite (createdAt, id) keyset)
+    const cur = (row?: BatchRow) => (row ? encodeBatchCursor(row) : undefined);
     let next: string | undefined;
     let previous: string | undefined;
     switch (direction) {
       case "forward":
-        previous = cursor ? batches.at(0)?.id : undefined;
+        previous = cursor ? cur(batches.at(0)) : undefined;
         if (hasMore) {
-          next = batches[pageSize - 1]?.id;
+          next = cur(batches[pageSize - 1]);
         }
         break;
       case "backward":
         batches.reverse();
         if (hasMore) {
-          previous = batches[1]?.id;
-          next = batches[pageSize]?.id;
+          previous = cur(batches[1]);
+          next = cur(batches[pageSize]);
         } else {
-          next = batches[pageSize - 1]?.id;
+          next = cur(batches[pageSize - 1]);
         }
         break;
     }
diff --git a/apps/webapp/test/batchListPresenter.readroute.test.ts b/apps/webapp/test/batchListPresenter.readroute.test.ts
@@ -309,9 +309,9 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
     }
   );
 
-  // Split scan merge serves new + legacy in one keyset-ordered page.
+  // Split scan merge serves new + legacy in one createdAt-ordered page; legacy is always read.
   heteroPostgresTest(
-    "split scan merges new (PG17) + legacy (PG14) rows under the keyset order; legacy read only when new does not fill the page",
+    "split scan merges new (PG17) + legacy (PG14) rows under the createdAt keyset order; legacy always read",
     async ({ prisma14, prisma17 }) => {
       const ctx14 = await seedParents(prisma14, "merge");
       await mirrorEnvParents(prisma17, ctx14, "merge");
@@ -323,7 +323,8 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
       await createBatch(prisma14, ctx14, { id: "batch_d", friendlyId: "fr_d", runCount: 4 });
       await createBatch(prisma17, ctx14, { id: "batch_e", friendlyId: "fr_e", runCount: 5 });
 
-      // Case A: small page fully served by new alone => legacy NOT read.
+      // Case A: always-merge — legacy is read even when new could fill the page (the old skip was
+      // unsound across the residency split). Page is the createdAt-ordered union of both DBs.
       const legacySpyA = spyClient(prisma14);
       const presenterA = new BatchListPresenter(prisma17, prisma17, {
         runOpsNew: prisma17,
@@ -332,9 +333,9 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
         splitEnabled: true,
       });
       const pageA = await presenterA.call(baseCall(ctx14, { pageSize: 2 }));
-      // new ids are e, c, a -> DESC: e, c (pageSize 2). pageSize+1 = 3 rows from new fills the page.
-      expect(pageA.batches.map((b) => b.id)).toEqual(["batch_e", "batch_c"]);
-      expect(legacySpyA.counts.findMany).toBe(0);
+      // union newest-first (createdAt, insertion order a<b<c<d<e): e, d, c, b, a -> page of 2 = e, d.
+      expect(pageA.batches.map((b) => b.id)).toEqual(["batch_e", "batch_d"]);
+      expect(legacySpyA.counts.findMany).toBeGreaterThan(0);
 
       // Case B: page needs legacy rows => legacy IS read and the merge is keyset-ordered union.
       const legacySpyB = spyClient(prisma14);
@@ -348,8 +349,8 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
       // union DESC of all 5: e, d, c, b, a -> first 4.
       expect(pageB.batches.map((b) => b.id)).toEqual(["batch_e", "batch_d", "batch_c", "batch_b"]);
       expect(legacySpyB.counts.findMany).toBeGreaterThan(0);
-      // cursor parity: next is the 4th id (pageSize-th), previous undefined (no input cursor).
-      expect(pageB.pagination.next).toBe("batch_b");
+      // cursor parity: next is the composite (createdAt, id) cursor of the 4th row, previous undefined.
+      expect(pageB.pagination.next?.endsWith("_batch_b")).toBe(true);
       expect(pageB.pagination.previous).toBeUndefined();
       expect(pageB.hasAnyBatches).toBe(true);
     }
@@ -436,7 +437,9 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
       const hasMore = direct.length > 2;
       const expectedPage = direct.slice(0, 2);
       expect(page.batches.map((b) => b.id)).toEqual(expectedPage.map((r) => r.id));
-      expect(page.pagination.next).toBe(hasMore ? expectedPage[1].id : undefined);
+      expect(
+        hasMore ? page.pagination.next?.endsWith(`_${expectedPage[1].id}`) : !page.pagination.next
+      ).toBe(true);
       expect(page.pagination.previous).toBeUndefined();
       expect(page.hasAnyBatches).toBe(true);
 
@@ -453,6 +456,87 @@ describe("BatchListPresenter run-ops read routing (PG14 control-plane/legacy + P
     }
   );
 
+  // REGRESSION: a flipped org's real id mix. A cuid ("c"=0x63) sorts ABOVE a run-ops id ("0"=0x30)
+  // under `id DESC`, so pre-flip legacy batches belong at the top — but #scanBatchTaskRun reads new
+  // first, skips legacy once the page is full, and `id < cursor` can never reach a "c…" from a "0…"
+  // cursor. Net: pre-flip legacy batches become unreachable.
+  heteroPostgresTest(
+    "flipped org: pre-flip legacy (cuid) batches remain reachable alongside post-flip run-ops batches",
+    async ({ prisma14, prisma17 }) => {
+      const ctx = await seedParents(prisma14, "flip");
+      await mirrorEnvParents(prisma17, ctx, "flip");
+
+      // Pre-flip cuid batch on legacy (sorts highest); post-flip run-ops batches on new (sort below).
+      const LEGACY_CUID = "cm0preflipbatch0000000001";
+      await createBatch(prisma14, ctx, { id: LEGACY_CUID, friendlyId: "fr_preflip", runCount: 9 });
+
+      const NEW_RUNOPS = [
+        "06fnewbatch00000000000000a",
+        "06fnewbatch00000000000000b",
+        "06fnewbatch00000000000000c",
+      ];
+      for (const id of NEW_RUNOPS) {
+        await createBatch(prisma17, ctx, { id, friendlyId: `fr_${id.slice(-1)}`, runCount: 1 });
+      }
+
+      const presenter = new BatchListPresenter(prisma17, prisma17, {
+        runOpsNew: prisma17,
+        runOpsLegacyReplica: prisma14,
+        controlPlaneReplica: prisma14,
+        splitEnabled: true,
+      });
+
+      // The pre-flip cuid batch is the oldest, so under newest-first it lands on a later page — but it
+      // must be REACHABLE by paging forward, not stranded behind the run-ops ids (the skip + id-order
+      // bug dropped it entirely: `id < <run-ops cursor>` never matches a "c…" id).
+      const seen = new Set<string>();
+      let cursor: string | undefined = undefined;
+      for (let i = 0; i < 10; i++) {
+        const page = await presenter.call(
+          baseCall(ctx, { pageSize: 2, cursor, direction: "forward" })
+        );
+        page.batches.forEach((b) => seen.add(b.id));
+        if (!page.pagination.next) break;
+        cursor = page.pagination.next;
+      }
+      expect([...seen]).toContain(LEGACY_CUID);
+    }
+  );
+
+  // REGRESSION (ordering): even with an always-merge fix, keyset-by-id is chronologically wrong across
+  // the flip — a cuid ("c") sorts above a run-ops id ("0"), so an OLDER pre-flip batch outranks a NEWER
+  // post-flip batch. The list is "newest first", so the later-created run-ops batch must come first.
+  heteroPostgresTest(
+    "flipped org: batches list is newest-first across the flip boundary (by createdAt, not id)",
+    async ({ prisma14, prisma17 }) => {
+      const ctx = await seedParents(prisma14, "order");
+      await mirrorEnvParents(prisma17, ctx, "order");
+
+      const OLD_LEGACY = "cm0oldbatch00000000000001"; // cuid, created EARLIER
+      const NEW_RUNOPS = "06fnewbatch000000000000001"; // run-ops, created LATER
+      await createBatch(prisma14, ctx, {
+        id: OLD_LEGACY,
+        friendlyId: "fr_old",
+        createdAt: new Date(Date.now() - 3_600_000),
+      });
+      await createBatch(prisma17, ctx, {
+        id: NEW_RUNOPS,
+        friendlyId: "fr_new",
+        createdAt: new Date(),
+      });
+
+      const presenter = new BatchListPresenter(prisma17, prisma17, {
+        runOpsNew: prisma17,
+        runOpsLegacyReplica: prisma14,
+        controlPlaneReplica: prisma14,
+        splitEnabled: true,
+      });
+      const page = await presenter.call(baseCall(ctx, { pageSize: 10 }));
+      // Newest-first: the later-created run-ops batch outranks the older legacy one.
+      expect(page.batches.map((b) => b.id)).toEqual([NEW_RUNOPS, OLD_LEGACY]);
+    }
+  );
+
   heteroRunOpsPostgresTest(
     "scan against dedicated RunOpsPrismaClient (splitEnabled): returns batches from new DB",
     async ({ prisma14, prisma17 }) => {
