fix(tables,knowledge): act on the claim outcome and scope liveness to the dispatch

Three defects, two of them created by the previous round's fixes.

Guarding the pending-to-dispatching claim without reading its outcome was the
worse half of a fix. When a Stop-all or the stale sweep won the race the row
correctly stayed `cancelled`, while the step went on to announce `dispatching`,
stamp cells and enqueue a window for it — and an empty window would then reach
the unguarded `markDispatchComplete` and overwrite `cancelled` with `complete`.
The step now ends when it did not claim the row.

The cell-liveness probe was table-scoped, and `table_row_executions` carries no
dispatch column, so a live dispatch's cells vouched for an abandoned dispatch
beside it and the abandoned row was never reclaimed — turning the stuck overlay
this sweep exists to clear into a permanent one. Narrowed to the dispatch's own
groups, which it already stores. Two active dispatches over the same groups can
still mask each other, but that is the state `markActiveDispatchesCancelled`
already prevents.

Truncation asks whether the window cut the sheet short — a question about the
declared range against the cap. Comparing the converted length to the cap made
it unreachable once the conversion was bounded; comparing the declared count to
the converted length then reported truncation for any sheet merely containing
blank rows, which are now skipped rather than defaulted into existence.

Author: waleedlatif1

apps/sim/lib/file-parsers/xlsx-parser.ts

@@ -177,12 +177,16 @@ export class XlsxParser implements FileParser {
         }
 
         /**
-         * Compared against the DECLARED row count, not the converted one. The
-         * conversion is now bounded to the preview window, so the converted
-         * length can never exceed it — comparing the two made this unreachable
-         * and silently dropped the notice from every sheet larger than the cap.
+         * Truncated means the WINDOW cut the sheet short, which is a question
+         * about the declared range against the cap — not about how many rows
+         * survived conversion. Comparing the converted length to the cap made
+         * this unreachable once the conversion was bounded (the two are equal by
+         * construction); comparing the declared count to the converted length
+         * instead reported truncation for any sheet merely containing blank
+         * rows, since those are now skipped. The CSV parser asks the same
+         * question the same way.
          */
-        if (rowCount > rowsToProcess) {
+        if (rowCount > CONFIG.MAX_PREVIEW_ROWS) {
           content += truncationNotice(
             `${rowCount.toLocaleString()} total rows, showing first ${rowsToProcess.toLocaleString()}`
           )

apps/sim/lib/file-parsers/xlsx-preview-bound.test.ts

@@ -79,4 +79,23 @@ describe('XlsxParser preview bound', () => {
     expect(result.metadata?.truncated).toBe(true)
     expect(result.content).toContain('200,000 total rows')
   })
+
+  /**
+   * A sheet whose declared range fits inside the window was not cut short, even
+   * though blank rows mean fewer rows survive conversion than the range names.
+   * Comparing the declared count against the converted length reported those as
+   * truncated.
+   */
+  it('does not report truncation for a small sheet containing blank rows', async () => {
+    // A genuinely empty row — empty strings are still cells and are not skipped.
+    const sheet = XLSX.utils.aoa_to_sheet([['header-a', 'header-b'], [], ['row-2-a', 'row-2-b']])
+    const book = XLSX.utils.book_new()
+    XLSX.utils.book_append_sheet(book, sheet, 'Sheet1')
+    const buffer = XLSX.write(book, { type: 'buffer', bookType: 'xlsx' }) as Buffer
+
+    const result = await new XlsxParser().parseBuffer(buffer)
+
+    expect(result.metadata?.truncated).toBe(false)
+    expect(result.content).not.toContain('total rows, showing first')
+  })
 })

apps/sim/lib/table/dispatcher.ts

@@ -405,7 +405,7 @@ export async function dispatcherStep(
   // user already saw the column flip to empty/Pending before any cell
   // started enqueueing.
   if (dispatch.status === 'pending') {
-    await db
+    const claimed = await db
       .update(tableRunDispatches)
       // Opens the heartbeat at the instant a holder takes the dispatch, so the
       // cleanup sweep ages it from that rather than from `requested_at`, which
@@ -424,6 +424,19 @@ export async function dispatcherStep(
           inArray(tableRunDispatches.status, [...ACTIVE_DISPATCH_STATUSES])
         )
       )
+      .returning({ id: tableRunDispatches.id })
+
+    /**
+     * Losing that race ends the step. Guarding the write without reading its
+     * outcome is the worse half of a fix: the row correctly stays `cancelled`,
+     * while this step goes on to announce `dispatching`, stamp cells and enqueue
+     * a window for it — and an empty window would then call the unguarded
+     * `markDispatchComplete` and overwrite `cancelled` with `complete`.
+     */
+    if (claimed.length === 0) {
+      logger.info(`[${dispatchId}] dispatch was cancelled before this step claimed it`)
+      return 'done'
+    }
     // Announce the dispatch the moment it starts — before the first window's
     // cells finish. Without this, auto-fired and capped dispatches (no client-
     // side optimistic seed) emit their first `dispatch` event only after window
@@ -844,6 +857,14 @@ export async function cancelStaleDispatches(
    * beating, nothing executing — is still collected. The subquery rides the
    * partial `(table_id, status)` index that already covers exactly these three
    * statuses.
+   *
+   * Narrowed to the dispatch's OWN groups, because `table_row_executions` has no
+   * dispatch column. Table-scoped, a live dispatch's cells would read as
+   * evidence that an abandoned dispatch beside it was still working, and the
+   * abandoned one would never be reclaimed — the stuck overlay this sweep exists
+   * to clear, now permanent. Two active dispatches over the SAME groups can
+   * still mask each other, but that is the state `markActiveDispatchesCancelled`
+   * already prevents: starting a run cancels prior work on its scope.
    */
   const isStale = () =>
     and(
@@ -852,6 +873,9 @@ export async function cancelStaleDispatches(
       sql`NOT EXISTS (
         SELECT 1 FROM ${tableRowExecutions}
         WHERE ${tableRowExecutions.tableId} = ${tableRunDispatches.tableId}
+          AND ${tableRowExecutions.groupId} IN (
+            SELECT jsonb_array_elements_text(${tableRunDispatches.scope} -> 'groupIds')
+          )
           AND ${tableRowExecutions.status} IN ('queued', 'running', 'pending')
           AND ${tableRowExecutions.updatedAt} >= ${sql.param(staleBefore, tableRowExecutions.updatedAt)}
       )`

apps/sim/lib/table/stale-dispatch-recovery.test.ts

@@ -114,6 +114,13 @@ describe('cancelStaleDispatches', () => {
     expect(chunks.some((chunk) => chunk.includes('NOT EXISTS'))).toBe(true)
     expect(chunks).toContain('tableRowExecutions.updatedAt')
     expect(chunks).toContain('tableRowExecutions.tableId')
+    /**
+     * Narrowed to the dispatch's own groups: row executions carry no dispatch
+     * column, so a table-wide probe lets a LIVE dispatch's cells vouch for an
+     * ABANDONED one beside it, and the abandoned row is never reclaimed.
+     */
+    expect(chunks).toContain('tableRowExecutions.groupId')
+    expect(chunks.some((chunk) => chunk.includes('jsonb_array_elements_text'))).toBe(true)
   })
 
   it('emits the terminal event so a stuck client overlay clears', async () => {
@@ -165,12 +172,33 @@ describe('dispatcherStep pending transition', () => {
    * resurrected the row, and with a fresh heartbeat the sweep would then wait
    * out another full window before reclaiming what it had already given up on.
    */
+  it('stops the step when the claim loses the race', async () => {
+    mockGetTableById.mockResolvedValue({
+      id: 'table-1',
+      schema: { workflowGroups: [{ id: 'group-1' }] },
+    })
+    dbChainMockFns.limit.mockResolvedValue([{ ...ABANDONED_ROW, status: 'pending' }])
+    // The guarded claim matched nothing — a Stop-all or the sweep got there first.
+    dbChainMockFns.returning.mockResolvedValue([])
+
+    const result = await dispatcherStep('tdsp_1')
+
+    /**
+     * Guarding the write without reading its outcome is the worse half of a fix:
+     * the row correctly stays cancelled while the step announces `dispatching`,
+     * stamps cells and enqueues a window for it.
+     */
+    expect(result).toBe('done')
+    expect(mockAppendTableEvent).not.toHaveBeenCalled()
+  })
+
   it('re-asserts the status it read before claiming the dispatch', async () => {
     mockGetTableById.mockResolvedValue({
       id: 'table-1',
       schema: { workflowGroups: [{ id: 'group-1' }] },
     })
     dbChainMockFns.limit.mockResolvedValue([{ ...ABANDONED_ROW, status: 'pending' }])
+    dbChainMockFns.returning.mockResolvedValue([{ id: 'tdsp_1' }])
 
     await dispatcherStep('tdsp_1').catch(() => {})