fix(tables): keep an emptied view terminated, and count masked reads off the seq scan

Author: waleedlatif1

apps/sim/hooks/queries/tables.ts

@@ -1300,6 +1300,14 @@ export function useDeleteTableRowsAsync({ workspaceId, tableId }: RowMutationCon
                   ...page,
                   rows: page.rows.filter((r) => keep.has(r.id)),
                   ...(page.totalCount != null ? { totalCount: keep.size } : {}),
+                  /**
+                   * The view is being emptied on purpose, so it has no next page — stated
+                   * explicitly because the server's cursor would otherwise say otherwise and
+                   * scrolling would pull back the very rows the job is deleting. Only the
+                   * row-count arithmetic used to carry this, which {@link hasMoreTableRows}
+                   * no longer consults once a cursor is present.
+                   */
+                  nextCursor: null,
                 })),
               }
             : old

apps/sim/hooks/queries/utils/table-rows-pagination.test.ts

@@ -75,10 +75,24 @@ describe('hasMoreTableRows', () => {
       expect(hasMoreTableRows(pages)).toBe(false)
     })
 
-    it('falls back to the count rules for pages cached before the cursor was threaded through', () => {
+    it('falls back to the count rules when a page carries no cursor', () => {
       expect(hasMoreTableRows([makePage(36, 100)])).toBe(true)
       expect(hasMoreTableRows([makePage(3, 3)])).toBe(false)
     })
+
+    /**
+     * The async "select all" delete strips rows from the active view and pins `nextCursor: null`
+     * so scrolling cannot pull back the rows the background job is still deleting. Deselecting a
+     * few leaves kept rows on the last page, so the row-count arithmetic that used to suppress
+     * `hasNextPage` no longer fires — only the pinned cursor does.
+     */
+    it('stays terminated for a partially-emptied view whose pages pin a null cursor', () => {
+      const pages = [
+        { ...makePage(2, 2), nextCursor: null },
+        { ...makePage(1, null, 2), nextCursor: null },
+      ]
+      expect(hasMoreTableRows(pages)).toBe(false)
+    })
   })
 })
 

apps/sim/hooks/queries/utils/table-rows-pagination.ts

@@ -10,8 +10,10 @@ interface TableRowsPageLike {
   rows: ReadonlyArray<{ id: string; orderKey?: string }>
   totalCount: number | null
   /**
-   * Optional because pages cached before this field was threaded through predate it; those fall
-   * back to the count rules below.
+   * Optional only so this loose page shape stays usable by callers that do not have a server
+   * response to hand (tests, and the optimistic mappings). On the wire it is required — the
+   * contract declares it non-optional and `requestJson` validates the response — so a real page
+   * always carries it and the count fallback below is defensive, not a live path.
    */
   nextCursor?: string | null
 }

apps/sim/lib/api/contracts/tables.test.ts

@@ -7,10 +7,14 @@ import { tableEventStreamQuerySchema, tableRowsQuerySchema } from '@/lib/api/con
 /**
  * `requestJson` parses the query through this schema on the CLIENT before building the URL, so
  * these values arrive as the caller's real types, not as URL strings. A string-only coercion
- * therefore read the grid's `includeTotal: param === 0` boolean as `false`, page 0 came back with
- * `totalCount: null`, and `hasMoreTableRows` — which treats a null total as "more may exist" —
- * reported `hasNextPage` forever. Every table then paid a wasted extra page fetch on mount and
- * before every row insert.
+ * therefore read the grid's `includeTotal: param === 0` boolean as `false`, and page 0 came back
+ * with `totalCount: null` on every table.
+ *
+ * What that broke is the **filtered** total: `rowTotal` was permanently null, so select-all and
+ * everything downstream of it (bulk delete, run scope, the selected-count label) silently fell
+ * back to the table's UNFILTERED `rowCount`. It also left `hasMoreTableRows` reading a null total
+ * as "more may exist" — though that half is now answered by `nextCursor` instead, so this schema
+ * is not what removes the wasted page fetch.
  */
 describe('tableRowsQuerySchema includeTotal', () => {
   it('accepts a real boolean, which is what the client passes', () => {

apps/sim/lib/api/contracts/tables.ts

@@ -806,7 +806,11 @@ export const tableRowsQueryBaseSchema = z.object({
    * gets its count. Everything else goes to {@link booleanQueryFlagSchema}, which accepts a real
    * boolean as well as the URL strings — `requestJson` parses this schema on the CLIENT before
    * building the URL, so the value arrives as the caller's own type, and a string-only coercion
-   * silently read the grid's `includeTotal: param === 0` as `false`.
+   * silently read the grid's `includeTotal: param === 0` as `false` — leaving `totalCount` null on
+   * every table, and select-all falling back to the unfiltered row count.
+   *
+   * Unparseable values now reject rather than resolving to `false`, matching `limit` and `offset`
+   * in this same schema, which have always thrown on garbage.
    */
   includeTotal: z
     .preprocess(

apps/sim/lib/table/planner.ts

@@ -16,14 +16,18 @@ const READ_STATEMENT_TIMEOUT_MS = 15_000
 const READ_LOCK_TIMEOUT_MS = 3_000
 
 /**
- * Applies every guard in ONE round-trip. Each `trx.execute` is its own serial round-trip (the
- * driver runs `prepare: false`), and every user-table read opens a transaction, so issuing these
- * separately cost 2–3 round-trips on every page, count, and drain batch.
+ * Applies every guard in ONE round-trip. Each awaited `trx.execute` is its own round-trip, and
+ * every user-table read opens a transaction, so issuing these separately cost 2–3 round-trips on
+ * every page, count, and drain batch.
  *
  * `set_config(name, value, is_local => true)` is exactly `SET LOCAL` — transaction-scoped, dying
- * with the commit — but it is a function call, so several fit in a single `SELECT`. Semicolon-
- * joining `SET LOCAL` statements would not work here: the driver sends this over the extended
- * protocol, which rejects multiple commands in one message.
+ * with the commit, and reverting the same way on a savepoint rollback — but it is a function call,
+ * so several fit in one `SELECT`. It also takes the values as bound parameters, which `SET LOCAL`
+ * cannot. That is the reason they must be one statement rather than semicolon-joined: a bound
+ * parameter forces the extended protocol, which rejects multiple commands per message.
+ *
+ * The guards are the first statement in the transaction, so an invalid value aborts it before
+ * `fn(trx)` can run — there is no path where a read proceeds unguarded.
  */
 async function setReadGuards(trx: DbTransaction, seqscanOff: boolean): Promise<void> {
   /**

apps/sim/lib/table/rows/service.ts

@@ -1152,7 +1152,13 @@ export async function queryRows(
   // unfiltered count already plans an index-only scan on the table_id prefix.
   // The count uses the full-view WHERE (no cursor seek): totals cover the whole
   // view, not the remaining pages.
-  const hasFilter = Boolean(userClause)
+  /**
+   * The delete mask counts as a filter: it injects JSONB predicates into `baseConditions`, which
+   * is exactly the plan shape `countRowsTenantBounded` exists to keep off a seq scan of the shared
+   * relation. Reading only `userClause` sent a masked-but-unfiltered count down the plain branch,
+   * bounded only by the statement timeout.
+   */
+  const hasFilter = Boolean(userClause || deleteMask)
   const countPromise = includeTotal
     ? hasFilter
       ? countRowsTenantBounded(whereClause)
@@ -1264,7 +1270,15 @@ interface BoundedFetchResult {
   anchorOffset: number
 }
 
-/** Belt-and-braces bound on drain iterations; unreachable in practice. */
+/**
+ * Belt-and-braces bound on drain iterations.
+ *
+ * Unreachable only because every iteration either consumes at least one row or cuts, and a bounded
+ * page's `limit` is capped at {@link TABLE_LIMITS.MAX_QUERY_LIMIT} — so the limit cut always fires
+ * first. That makes the two constants exactly tight: raising `MAX_QUERY_LIMIT` above this bound
+ * would let the loop exit with rows still unread and `hasMore: false`, which clients now trust as
+ * end-of-table (they terminate on `nextCursor`, which this decides). Raise both together.
+ */
 const MAX_QUERY_BATCHES = 1000
 
 /**