review(tables): drop the delete-mask elision and correct the provenance snapshot doc

Reading the delete job from the table a request already loaded widened a race
the mask probe has always had — a job committing between the check and the row
read is missed either way, but trusting the loaded fields moves the check two
queries earlier. Closing it properly means evaluating the job inside the row
read's own snapshot, which is a larger change than this one, so the elision is
removed and `pending-delete-mask.ts` is back to what it was.

The three remaining reductions are untouched: they were the bulk of the win,
and each is a read this code cannot need rather than a read it takes on faith.

Also updates `TableRowProvenanceReader`'s doc, which still described one
repeatable-read transaction per batch.

Author: waleedlatif1

apps/sim/app/api/v1/tables/[tableId]/rows/route.ts

@@ -185,8 +185,6 @@ export const GET = withRouteHandler(async (request: NextRequest, context: TableR
         offset: validated.offset,
         includeTotal: validated.includeTotal,
         withExecutions: false,
-        // `table` was loaded a few lines above, for this read.
-        trustLoadedJob: true,
       },
       requestId
     )

apps/sim/lib/table/__tests__/service-filter-threading.test.ts

@@ -670,66 +670,17 @@ describe('queryRows byte budget', () => {
 })
 
 /**
- * Two reads the row path used to make unconditionally, each one round trip on the grid's hot
- * page. Both are now answered from state the caller already holds; these pin that the query is
- * actually skipped rather than merely ignored, and that the fallbacks still run.
+ * The run-state sidecar read the row path used to make unconditionally, one round trip (four on a
+ * full page) for tables that cannot hold a single sidecar row. This pins that the query is
+ * actually skipped rather than merely ignored, and that the fallback still runs.
  */
-describe('queryRows round-trip elision', () => {
+describe('queryRows run-state elision', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     resetDbChainMock()
     vi.mocked(tableMayHaveRunState).mockReturnValue(true)
   })
 
-  const hydrated = (
-    fields: Partial<Pick<TableDefinition, 'jobStatus' | 'jobType'>>
-  ): TableDefinition => ({ ...TABLE, jobStatus: null, jobType: null, ...fields })
-
-  it('skips the delete-job probe when the hydrated table shows no running delete', async () => {
-    await queryRows(
-      hydrated({}),
-      { limit: 5, includeTotal: false, withExecutions: false, trustLoadedJob: true },
-      'req-1'
-    )
-
-    // With no probe, the drain batch is the FIRST bounded query rather than the second.
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(1, 6)
-  })
-
-  it('still probes when the hydrated table shows a running delete job', async () => {
-    await queryRows(
-      hydrated({ jobStatus: 'running', jobType: 'delete' }),
-      { limit: 5, includeTotal: false, withExecutions: false, trustLoadedJob: true },
-      'req-1'
-    )
-
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(1, 1)
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(2, 6)
-  })
-
-  /** An unhydrated definition cannot rule the job out, so it keeps the lookup it always had. */
-  it('still probes when the table carries no job fields at all', async () => {
-    await queryRows(
-      TABLE,
-      { limit: 5, includeTotal: false, withExecutions: false, trustLoadedJob: true },
-      'req-1'
-    )
-
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(1, 1)
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(2, 6)
-  })
-
-  /**
-   * A caller that holds its table across a long walk (the export stream) must keep re-asking, so
-   * a delete job starting mid-walk still begins masking its doomed rows.
-   */
-  it('still probes for a caller that did not vouch for its table', async () => {
-    await queryRows(hydrated({}), { limit: 5, includeTotal: false, withExecutions: false }, 'req-1')
-
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(1, 1)
-    expect(dbChainMockFns.limit).toHaveBeenNthCalledWith(2, 6)
-  })
-
   it('skips the run-state read for a table that can hold none, still reporting empty executions', async () => {
     vi.mocked(tableMayHaveRunState).mockReturnValue(false)
     dbChainMockFns.limit.mockResolvedValueOnce([])

apps/sim/lib/table/application/rows.test.ts

@@ -769,7 +769,6 @@ describe('row query and upsert application semantics', () => {
         includeTotal: false,
         withExecutions: false,
         runStateBudgetBytes: TABLE_LIMITS.MAX_ROW_RUN_STATE_BYTES,
-        trustLoadedJob: true,
       },
       expect.any(String)
     )

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

@@ -436,8 +436,6 @@ export const listTableRows = defineAuthorizedTableUseCase({
           includeTotal: false,
           withExecutions: input.includeRunState ?? false,
           runStateBudgetBytes: TABLE_LIMITS.MAX_ROW_RUN_STATE_BYTES,
-          // `context.table` was loaded by this use case's own resolver, for this read.
-          trustLoadedJob: true,
         },
         requestId(input)
       )
@@ -575,8 +573,6 @@ export const queryTableRows = defineAuthorizedTableUseCase({
           withExecutions: input.includeRunState ?? false,
           runStateBudgetBytes: TABLE_LIMITS.MAX_ROW_RUN_STATE_BYTES,
           columnIds,
-          // `context.table` was loaded by this use case's own resolver, for this read.
-          trustLoadedJob: true,
         },
         requestId(input),
         readProvenance

apps/sim/lib/table/rows/pending-delete-mask.ts

@@ -9,43 +9,6 @@ import type { TableDefinition, TableDeleteJobPayload } from '@/lib/table/types'
 
 const logger = createLogger('TablePendingDeleteMask')
 
-/**
- * Whether {@link PendingDeleteMaskOptions.trustLoadedJob} can rule a running delete job out.
- *
- * Every table loaded through `getTableById` / `listTables` already carries its latest non-export
- * job, folded into that same SELECT as a lateral, so a caller that loaded its table for this very
- * read already holds the answer and the lookup is pure overhead on the hot path.
- *
- * The derivation is exact, not a heuristic. `table_jobs_one_active_per_table` is unique on
- * `table_id WHERE status = 'running' AND type <> 'export'` — the same predicate the lateral
- * filters on — so a running delete job is the ONLY running non-export job on its table, and no
- * further non-export job can be inserted while it holds that slot. It is therefore the newest
- * non-export job by `started_at`, which is exactly the row the lateral returns.
- *
- * `jobStatus === undefined` means the fields were never hydrated (a `TableDefinition` assembled
- * by some other path), which is indistinguishable from "no job" in the shape alone — so that case
- * falls back to the query rather than assuming. A hydrated table with no job has
- * `jobStatus: null`.
- */
-function hydratedJobRulesOutDelete(table: TableDefinition): boolean {
-  if (table.jobStatus === undefined) return false
-  return !(table.jobStatus === 'running' && table.jobType === 'delete')
-}
-
-export interface PendingDeleteMaskOptions {
-  /**
-   * Answer from `table`'s own latest-job fields when they rule a running delete out, instead of
-   * querying for one.
-   *
-   * Only for a caller whose `table` was loaded for this read: the fields are then as fresh as the
-   * query would have been. A caller that loads a table once and then pages for a while — the
-   * export runner, the snapshot builder — must NOT set this, because a delete job starting
-   * mid-walk would never appear in its snapshot and its later pages would stop masking doomed
-   * rows. Those callers keep re-asking per page, which is what makes the mask appear mid-walk.
-   */
-  trustLoadedJob?: boolean
-}
-
 /**
  * Visibility mask for a running delete job: returns a clause keeping only rows the job will NOT
  * delete, or `undefined` when no delete job is running. The job's persisted scope
@@ -57,11 +20,7 @@ export interface PendingDeleteMaskOptions {
  * `(doomed) IS NOT TRUE` rather than `NOT (doomed)`: JSONB predicates evaluate to NULL on missing
  * cells, and those rows are NOT selected for deletion (NULL ≠ TRUE) — they must stay visible.
  */
-export async function pendingDeleteMask(
-  table: TableDefinition,
-  options?: PendingDeleteMaskOptions
-): Promise<SQL | undefined> {
-  if (options?.trustLoadedJob && hydratedJobRulesOutDelete(table)) return undefined
+export async function pendingDeleteMask(table: TableDefinition): Promise<SQL | undefined> {
   const [job] = await db
     .select({ payload: tableJobs.payload })
     .from(tableJobs)

apps/sim/lib/table/rows/secret-provenance.ts

@@ -959,8 +959,10 @@ export async function loadTableRowSecretProvenance(
 
 /**
  * Collects only returned row values while their database snapshot is still valid.
- * Readers use one repeatable-read transaction per bounded batch; writers capture
- * after stamping and before releasing row locks. Nothing is reloaded after commit.
+ * A read captures inside one repeatable-read transaction spanning every batch of
+ * its page, so a row and the sidecar captured for it always come from the same
+ * snapshot; writers capture after stamping and before releasing row locks.
+ * Nothing is reloaded after commit.
  */
 export class TableRowProvenanceReader {
   private readonly accumulator: ResolvedSecretTraceProvenanceAccumulator

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

@@ -1042,9 +1042,7 @@ export async function findRowMatches(
   if (columnIds.length === 0) return { matches: [], truncated: false }
 
   // Same visibility rule as queryRows: don't surface rows a running delete job will remove.
-  // Search is always one shot against a table its caller just loaded, so the table's own
-  // latest-job fields are as fresh as a lookup would be — see `PendingDeleteMaskOptions`.
-  const deleteMask = await pendingDeleteMask(table, { trustLoadedJob: true })
+  const deleteMask = await pendingDeleteMask(table)
 
   const baseConditions = and(
     eq(userTableRows.tableId, table.id),
@@ -1164,15 +1162,14 @@ export async function queryRows(
     withExecutions = true,
     runStateBudgetBytes,
     columnIds,
-    trustLoadedJob = false,
   } = options
 
   const tableName = USER_TABLE_ROWS_SQL_NAME
   const columns = table.schema.columns
 
   // Hide rows a running delete job is about to remove — both the page and the count below share
   // this clause, so totals stay consistent with the visible rows.
-  const deleteMask = await pendingDeleteMask(table, { trustLoadedJob })
+  const deleteMask = await pendingDeleteMask(table)
 
   const baseConditions = and(
     eq(userTableRows.tableId, table.id),

apps/sim/lib/table/types.ts

@@ -626,16 +626,6 @@ export interface QueryOptions {
    *  Mutually exclusive with `sort`. May be combined with `offset` (a compound
    *  cursor seeks the anchor, then offsets past unkeyed rows consumed after it). */
   after?: TableRowsCursor
-  /**
-   * Answer "is a delete job running?" from `table`'s own latest-job fields rather than querying
-   * for one, saving a round trip on every read.
-   *
-   * Set it only when `table` was loaded for this very read — a request handler that resolved its
-   * table and is about to return. A caller that loads a table once and then pages for a while
-   * (the export stream) must leave it off, so a delete job starting mid-walk still begins masking
-   * its doomed rows partway through.
-   */
-  trustLoadedJob?: boolean
   /**
    * When true (default), runs a `COUNT(*)` and returns `totalCount` as a number.
    * Pass `false` to skip the count query (grid UI doesn't need it); `totalCount`