feat(tables): paginated background row-delete jobs via table_jobs#4915
feat(tables): paginated background row-delete jobs via table_jobs#4915TheodoreSpeaks wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Background delete: New Also in this PR: Async export (upload + presigned download), async backfill for large output columns, optional Trigger.dev tasks for import/delete/export/backfill, cron janitor for stale Reviewed by Cursor Bugbot for commit 3f8a67a. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR adds a background row-delete job (paginated, filter-scoped, ownership-gated) alongside a new
Confidence Score: 4/5Safe to merge after addressing the incorrect row count shown in the delete confirmation dialog when a filter is active. The backend (migration, delete runner, job CRUD, concurrency gate) is well-designed and correct. The one defect is in the frontend: the delete confirmation dialog shows the total table row count instead of the filtered count when a filter is active, so a user deleting 50 filtered rows sees 'Delete 10,000 rows…'. The over-broad optimistic cache clear is a minor multi-cached-state edge case. Everything else — keyset pagination, cutoff semantics, ownership checks, SSE reconciliation, import compatibility — looks solid. apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx (estimatedCount logic) and apps/sim/hooks/queries/tables.ts (onMutate query scope) Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant DeleteAsyncRoute as POST /delete-async
participant DB as PostgreSQL
participant Worker as runTableDelete (detached)
participant SSE as SSE stream
Client->>DeleteAsyncRoute: "POST {filter, excludeRowIds}"
DeleteAsyncRoute->>DB: markTableJobRunning (INSERT table_jobs ON CONFLICT DO NOTHING)
alt slot already taken
DB-->>DeleteAsyncRoute: conflict 0 rows
DeleteAsyncRoute-->>Client: 409 A job is already in progress
else slot claimed
DB-->>DeleteAsyncRoute: job row inserted
DeleteAsyncRoute->>Worker: runDetached(runTableDelete)
DeleteAsyncRoute-->>Client: "200 {jobId}"
loop each page
Worker->>DB: updateJobProgress (heartbeat + ownership check)
alt job canceled
DB-->>Worker: 0 rows JobSupersededError stop
end
Worker->>DB: "selectRowIdPage (id > afterId, createdAt <= cutoff, filter)"
DB-->>Worker: page of IDs
Worker->>DB: deletePageByIds skipCompaction
Worker->>SSE: appendTableEvent(running, progress) every 5000 rows
end
Worker->>DB: markJobReady
Worker->>SSE: appendTableEvent(ready)
SSE-->>Client: invalidate rows + detail query
end
opt user cancels
Client->>DB: "POST /job/cancel {jobId}"
DB-->>Client: "canceled=true"
SSE-->>Client: appendTableEvent(canceled) restore optimistic rows
end
Reviews (1): Last reviewed commit: "feat(tables): paginated background row-d..." | Re-trigger Greptile |
| const excludeRowIds = rowSel.excluded ? [...rowSel.excluded] : [] | ||
| const estimatedCount = Math.max(0, tableRowCountRef.current - excludeRowIds.length) | ||
| onRequestDeleteAllByFilter({ excludeRowIds, estimatedCount }) |
There was a problem hiding this comment.
estimatedCount uses tableRowCountRef.current, which is the TOTAL table row count (tableData.rowCount), not the count matching the active filter. When a filter is active, the confirmation dialog will say "Delete 10,000 rows matching the current filter" even though only a small subset of those rows match the filter and will actually be deleted. This could alarm users or make them distrust the UI. The filtered count should come from the rows query's metadata (or at minimum the dialog should not show a count when a filter is active).
| const excludeRowIds = rowSel.excluded ? [...rowSel.excluded] : [] | |
| const estimatedCount = Math.max(0, tableRowCountRef.current - excludeRowIds.length) | |
| onRequestDeleteAllByFilter({ excludeRowIds, estimatedCount }) | |
| const excludeRowIds = rowSel.excluded ? [...rowSel.excluded] : [] | |
| // When a filter is active, tableRowCountRef.current is the *total* table count, not the | |
| // filtered count — pass undefined so the dialog avoids showing a misleading number. | |
| const estimatedCount = queryOptions?.filter | |
| ? undefined | |
| : Math.max(0, tableRowCountRef.current - excludeRowIds.length) | |
| onRequestDeleteAllByFilter({ excludeRowIds, estimatedCount }) |
| await queryClient.cancelQueries({ queryKey: tableKeys.rowsRoot(tableId) }) | ||
| const previousQueries = queryClient.getQueriesData<InfiniteData<TableRowsResponse, number>>({ | ||
| queryKey: tableKeys.rowsRoot(tableId), | ||
| }) | ||
| const previousDetail = queryClient.getQueryData<TableDefinition>(tableKeys.detail(tableId)) | ||
| const keep = new Set(excludeRowIds ?? []) | ||
| queryClient.setQueriesData<InfiniteData<TableRowsResponse, number>>( | ||
| { queryKey: tableKeys.rowsRoot(tableId), exact: false }, | ||
| (old) => | ||
| old | ||
| ? { | ||
| ...old, | ||
| pages: old.pages.map((page) => ({ | ||
| ...page, | ||
| rows: page.rows.filter((r) => keep.has(r.id)), | ||
| })), | ||
| } | ||
| : old |
There was a problem hiding this comment.
setQueriesData with exact: false updates every cached row query for this table, not just the one that matches the current filter. If the user previously loaded the table without a filter (or with a different filter), those cached pages are also cleared. Since onSuccess only invalidates tableKeys.lists() (not row queries), the stale no-op rows stay empty until the SSE terminal event fires — which may be minutes later for a large delete. The fix is to restrict the optimistic update to the exact query key that matches the current filter.
…ed optimistic clear, Cmd+A select-all, hide delete from tray)
…row-deletes # Conflicts: # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx # apps/sim/app/workspace/[workspaceId]/tables/components/import-csv-dialog/import-csv-dialog.tsx # apps/sim/app/workspace/[workspaceId]/tables/tables.tsx # apps/sim/lib/table/import-runner.ts # apps/sim/lib/table/service.ts # packages/db/migrations/meta/0227_snapshot.json # packages/db/migrations/meta/_journal.json
| const lockedId = prev?.importId | ||
| if (lockedId && importId && importId !== lockedId) return | ||
| const lockedId = prev?.jobId | ||
| if (lockedId && jobId && jobId !== lockedId) return |
There was a problem hiding this comment.
Terminal job events dropped early
High Severity
applyJob ignores terminal SSE events when the table detail cache has no jobId, but async delete clears rows in onMutate and only sets jobId in onSuccess. If the worker finishes and emits ready or failed before that lands, the event is dropped, row invalidation never runs, and a failed delete can leave the grid empty while data still exists.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b324ac0. Configure here.
…sk, keyset index + autovacuum tuning
| .orderBy(desc(tableJobs.startedAt)) | ||
| .limit(1) | ||
| return mapJobRow(row) | ||
| } |
There was a problem hiding this comment.
Sync import shows stale job
Medium Severity
After a successful synchronous CSV import, releaseJobClaim removes the transient running row from table_jobs without recording success. latestJobForTable then surfaces the previous terminal job (often failed with an old error), so the table can look like a background job failed even though the sync import succeeded.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 03c98b0. Configure here.
| } | ||
| if (rowIds.length === 0) return null | ||
| return { groupIds: [...groupIdsInRect], rowIds, allRows: false } | ||
| return { groupIds: [...groupIdsInRect], rowIds, allRows: false, rowCount: rowIds.length } |
There was a problem hiding this comment.
Select-all count stays stale
Medium Severity
For a filter-scoped select-all, selectedRunScope.rowCount reads selectAllTotalRef, which tracks rowTotal from the server, but the useMemo that builds selectedRunScope omits rowTotal and tableData?.rowCount from its dependency list. If the user selects all before the filtered count arrives, the action bar and run scope keep the unfiltered table count until selection changes.
Reviewed by Cursor Bugbot for commit 03c98b0. Configure here.
…ith in-process fallback
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3f8a67a. Configure here.
| } | ||
| if (rowIds.length === 0) return null | ||
| return { groupIds: [...groupIdsInRect], rowIds, allRows: false } | ||
| return { groupIds: [...groupIdsInRect], rowIds, allRows: false, rowCount: rowIds.length } |
There was a problem hiding this comment.
Select-all scope stays stale
Medium Severity
selectedRunScope for kind: 'all' reads queryOptions.filter and selectAllTotalRef but the useMemo omits queryOptions and rowTotal, so a filter change with select-all still active can keep the old filter for Play/Refresh. The selection snapshot effect treats run scope as unchanged when only rowCount, filter, or allRows differ, so the action bar can show a wrong cell count after the filter-scoped total loads.
Reviewed by Cursor Bugbot for commit 3f8a67a. Configure here.
Summary
{ filter, excludeRowIds }instead of every row id, deleting in keyset-paginated batches. Rows inserted mid-job are spared via acreated_atcutoff.table_jobstable (one row per job,type= import|delete). Concurrency gate is a partial-unique index on(table_id) WHERE status='running'— one bg job per table, so import/delete are mutually exclusive. Job fields are re-exposed onTableDefinitionvia a latest-job join, so the tray/SSE/grid stay unchanged.{ kind:'all', excluded }so "select all then deselect" maps cleanly to the job — fixes the old bug where deselect-after-select-all silently dropped unloaded rows.0227: createtable_jobs, backfill in-flight import state, drop the oldimport_*columns. Janitor sweeps stale running jobs → failed and prunes terminal jobs.Type of Change
Testing
bun run lintclean,bun run check:api-validation:strictpasses,tsc --noEmitclean, 260 table/cron tests pass (added delete-runner, delete-async, and job/cancel tests).Checklist