Skip to content

fix(tables): fix bulk ops truncation for tables larger than one page#4532

Merged
waleedlatif1 merged 20 commits intostagingfrom
fix/tabls
May 9, 2026
Merged

fix(tables): fix bulk ops truncation for tables larger than one page#4532
waleedlatif1 merged 20 commits intostagingfrom
fix/tabls

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • Bulk ops (column clear, copy, cut, delete, run) were silently truncated to 1000 rows on tables with more than one page of data
  • Added background page drain via `useEffect` + `fetchNextPage` so all pages load automatically after mount
  • Added `ensureAllRowsLoaded` helper that gates bulk ops — awaits any remaining pages before proceeding
  • Fixed undo/redo regression: batch update mutations now chunk by `MAX_BULK_OPERATION_SIZE` to stay within server limits on large tables
  • Added new `tableRowsInfiniteOptions` factory so the hook and imperative drain calls share the same cache key
  • Added tests for chunking logic in `use-table-undo` and drain logic in `use-table`

Type of Change

  • Bug fix

Testing

Tested manually — verified all rows clear/copy/cut/delete correctly on tables >1000 rows. All unit tests passing.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment May 9, 2026 8:07pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 9, 2026

PR Summary

Medium Risk
Touches table-grid bulk operations (copy/cut/clear/delete/run) and undo/redo batching, adding async page-draining and chunked mutations; regressions could affect large-table edits and clipboard behavior. No auth/security changes.

Overview
Fixes bulk table operations silently stopping at the first 1000 loaded rows by adding useTable.ensureAllRowsLoaded() (drains React Query infinite pages via a shared tableRowsInfiniteOptions cache key) and gating bulk clear/copy/cut/delete flows on it.

Updates bulk mutations to respect server batch limits by chunking batchUpdate calls (including undo/redo via an async runWithoutRecording) and adds targeted unit tests for the drain/chunk logic.

Smaller UX/validation tweaks: string cells auto-render valid URLs as external links with favicons (adds tldts), number cleaning now yields null instead of 0 on invalid input, CSV-imported table names are truncated to TABLE_LIMITS.MAX_TABLE_NAME_LENGTH (which is increased to 128), and CSV import errors now toast the server message.

Reviewed by Cursor Bugbot for commit 290b4bf. Configure here.

Comment thread apps/sim/hooks/use-table-undo.ts Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 9, 2026

Greptile Summary

This PR fixes bulk-operation truncation on large tables (>1 page) by adding ensureAllRowsLoaded — a React Query cache drain that fetches all remaining pages before clear, copy, cut, and delete ops execute. It also introduces chunked batch mutations (chunkBatchUpdates) to stay within server size limits, upgrades runWithoutRecording to async so the undo guard spans the full operation, and surfaces previously-silent CSV import errors via onError toasts.

  • ensureAllRowsLoaded: shares tableRowsInfiniteOptions query key with the hook so cache reads are coherent; called via ref in all bulk keyboard handlers before building mutation payloads.
  • chunkBatchUpdates: splits updates into MAX_BULK_OPERATION_SIZE chunks dispatched 3-at-a-time; used in table-grid.tsx op handlers and undo/redo in use-table-undo.ts.
  • selectedRunScope.allRows: header-checkbox selection now propagates allRows: true to table.tsx, which passes rowIds: undefined to the server — but the sameRunScope snapshot equality check does not compare allRows, so this fix doesn't take effect when transitioning from a fully-manually-selected page to "select all".

Confidence Score: 3/5

Not safe to merge as-is: the run-scope fix is incomplete and the header-checkbox 'run all rows' path remains truncated to the loaded page.

The sameRunScope check in table-grid.tsx (lines 2859-2867) compares groupIds and rowIds element-by-element but skips allRows. A user who manually selects all loaded rows then clicks the header checkbox produces identical rowIds arrays with different allRows values; the snapshot is not re-emitted; table.tsx retains allRows: false; and runScope is called with explicit row IDs rather than undefined, capping the run to the first page — the truncation the PR set out to fix.

table-grid.tsx — sameRunScope equality check needs allRows comparison; cell-render.tsx — Google favicon service makes external requests for every URL-valued cell

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx Bulk ops (clear, copy, cut) now correctly drain all pages via ensureAllRowsLoaded; selectedRunScope gains allRows: true for header-checkbox selections; but sameRunScope equality check does not compare allRows, so a transition from manually-selecting all loaded rows to header checkbox won't re-emit the snapshot and table.tsx retains stale allRows: false, truncating the run
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts Adds ensureAllRowsLoaded: reads from React Query cache synchronously after each fetchNextPage to drain all pages; correctly shares the tableRowsInfiniteOptions query key with useInfiniteTableRows to ensure cache coherence
apps/sim/hooks/queries/tables.ts Extracts tableRowsInfiniteOptions factory to share query key between hook and imperative drain; adds toast.error to CSV upload onError callbacks to surface previously-silent validation failures
apps/sim/hooks/use-table-undo.ts Batch update mutations now chunk by MAX_BULK_OPERATION_SIZE to stay within server limits; executeAction is async so runWithoutRecording guards the full operation; the delete-column undo path's cell-restore batch runs in a fire-and-forget IIFE inside onSuccess, so errors are only logged with no user toast
apps/sim/stores/table/store.ts runWithoutRecording upgraded from sync to async: undoRedoInProgress flag now stays true until the returned Promise settles, correctly guarding chunked batch mutations
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx Adds URL auto-detection for string columns: validates with new URL() or BARE_DOMAIN_RE+tldts.isIcann, renders clickable links with Google favicon service; favicon requests leak cell domains to an external service
apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx onPlay/onRefresh now pass rowIds: undefined when allRows: true, correctly requesting a server-side full-table run; stop correctly calls onStopAll() for all-row selections
apps/sim/lib/table/constants.ts MAX_TABLE_NAME_LENGTH raised from 50 to 128; adds MAX_BULK_OPERATION_SIZE constant that gates chunking across multiple call sites

Sequence Diagram

sequenceDiagram
    participant User
    participant TableGrid
    participant ensureAllRowsLoaded
    participant ReactQueryCache
    participant Server

    User->>TableGrid: Bulk op (Delete/Clear/Copy/Cut) on row selection
    TableGrid->>ensureAllRowsLoaded: await ensureAllRowsLoadedRef.current()
    loop "until lastPage.rows.length < MAX_QUERY_LIMIT"
        ensureAllRowsLoaded->>ReactQueryCache: getQueryData(queryKey)
        ensureAllRowsLoaded->>Server: fetchNextPage()
        Server-->>ReactQueryCache: page N rows
    end
    ensureAllRowsLoaded-->>TableGrid: all rows[]
    TableGrid->>TableGrid: pushUndo(undoCells)
    TableGrid->>Server: chunkBatchUpdates (3 concurrent chunks)
    Server-->>TableGrid: success / error
Loading

Comments Outside Diff (1)

  1. apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx, line 2859-2867 (link)

    P1 sameRunScope omits allRows — stale flag silently truncates run

    sameRunScope compares groupIds and rowIds element-by-element but never checks the new allRows field. When a user manually checks all loaded rows (rowSelection.kind === 'some', allRows: false) and then clicks the header checkbox to "select all" (rowSelection.kind === 'all', allRows: true), both states produce a selectedRunScope with the same rowIds array. sameRunScope returns true, the snapshot is not re-emitted, and table.tsx retains the stale allRows: false. onPlay/onRefresh then call runScope with explicit row IDs instead of rowIds: undefined, silently truncating the run to the loaded page — the exact bug this PR claims to fix.

    Add && prev.selectedRunScope.allRows === selectedRunScope.allRows to the equality check.

Reviews (8): Last reviewed commit: "feat(tables): clickable URL cells with f..." | Re-trigger Greptile

Comment thread apps/sim/hooks/queries/tables.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

Comment thread apps/sim/hooks/queries/tables.ts Outdated
Comment thread apps/sim/hooks/queries/tables.ts
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a89ee35. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2ba4836. Configure here.

Bulk operations (column-header delete, select-all copy/cut/delete/run)
were silently truncated to the first 1000 rows because handlers only
iterated the loaded pages from useInfiniteQuery.

Fix:
- Extract tableRowsInfiniteOptions factory (infiniteQueryOptions) so
  the hook and imperative drain share the same typed cache key
- Add background drain via useEffect watching hasNextPage/isFetchingNextPage
  — chains fetchNextPage until getNextPageParam returns undefined
- Add ensureAllRowsLoaded to use-table: reads cache via getQueryData +
  calls fetchNextPage in a while loop until the last page is partial
- Await ensureAllRowsLoaded at every kind:'all' bulk-op entry point in
  table-grid (column delete, copy, cut, action-bar delete/run)
- Add chunkBatchUpdates to send updates in MAX_BULK_OPERATION_SIZE=1000
  chunks so server validation never rejects oversized batches
- Fix undo-redo: make executeAction async and chunk clear-cells,
  update-cells, and delete-column cell-restore with mutateAsync loops

Tests: 41 passing across use-table, tables queries, and use-table-undo
…entity

- Fix polling tick: move Promise.all inside else-branch so dirty[] stays in
  scope; keep hasDirty=true during active mutations so the short interval
  fires while chunked batch-updates are in flight
- Add isColumnSelectionRef branch to handleCopy (mirrors handleCut fix):
  column-header Cmd+C now drains all pages before building clipboard content
- Replace String(updatedAt) comparison in mergePagePreservingIdentity with
  Date.getTime() equality — handles ISO vs +00:00 timezone variants
- Remove redundant batchUpdates.length > 0 guards at chunkBatchUpdates
  callsites (empty-array case is handled inside the function)
- Export _mergePagePreservingIdentity for unit testing
- Add 6 unit tests covering mergePagePreservingIdentity edge cases
…ch updates

- Drop the per-page polling loop — SSE stream already patches execution
  cell state in real time and invalidates on buffer prune; polling was
  redundant and burned CPU/network on every open table
- Remove eager mount drain (fetchNextPage loop in use-table.ts); scroll
  handler and ensureAllRowsLoaded handle progressive/on-demand loading
- Parallelize chunkBatchUpdates with a 3-worker pool instead of serial
  chunks, reducing bulk-op round-trips by ~3x
- Delete mergePagePreservingIdentity and its tests (no longer called)
- Fix chunkBatchUpdates JSDoc to reflect parallel dispatch (was "sequentially")
- Inline CHUNK_CONCURRENCY=3, single-use constant needs no abstraction
- Drop stale "Polls while any cell is in flight" from useTableRows JSDoc
- Remove two generic "Validation errors surfaced by caller" comments
- Remove ASCII separator line from workflow group mutations section
- Remove dead `if (!variables) return` guard in useImportCsvIntoTable onSettled
  (TanStack v5 always provides variables to onSettled)
…ded pages

When rowSelection.kind === 'all', selectedRunScope now flags allRows: true.
The action-bar run handlers pass rowIds: undefined to the server when allRows
is set, matching the server contract (missing rowIds = run all eligible rows).
Stop likewise routes through scope: 'all' instead of per-row cancels.

Previously, selecting all rows and clicking Run would silently only run the
rows loaded in the current infinite-query cache (potentially one page of 1000
on a 5000-row table).
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 290b4bf. Configure here.

@waleedlatif1 waleedlatif1 merged commit c7130c6 into staging May 9, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/tabls branch May 9, 2026 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant