From 40cb72a41b4335d62ae8b6d8eda4dbbc04231007 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 11 Nov 2026 10:24:16 -0800 Subject: [PATCH 1/2] fix(tables): show rejected filter errors in the grid and fill imports to the row limit --- .../components/table-grid/table-grid.tsx | 207 +++++++++--------- .../tables/[tableId]/hooks/use-table.ts | 12 + apps/sim/lib/table/import-runner.test.ts | 118 ++++++++++ apps/sim/lib/table/import-runner.ts | 49 ++++- 4 files changed, 276 insertions(+), 110 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 88681d3a975..a6bd0c37aa8 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -590,6 +590,7 @@ export function TableGrid({ rows, rowTotal, isLoadingRows, + rowsError, fetchNextPage, hasNextPage, isFetchingNextPage, @@ -4973,104 +4974,114 @@ export function TableGrid({ )} - {isLoadingTable || isLoadingRows - ? null - : (() => { - const virtualItems = rowVirtualizer.getVirtualItems() - // `item.start`/`item.end` include `scrollMargin` (the sticky-header - // offset) but `getTotalSize()` already nets it out, so both spacer - // heights are computed relative to `scrollMargin`. - const scrollMargin = rowVirtualizer.options.scrollMargin - const paddingTop = - virtualItems.length > 0 ? virtualItems[0].start - scrollMargin : 0 - const paddingBottom = - virtualItems.length > 0 - ? rowVirtualizer.getTotalSize() - - (virtualItems[virtualItems.length - 1].end - scrollMargin) - : 0 - return ( - <> - {paddingTop > 0 && ( - - - - )} - {virtualItems.map((virtualRow) => { - const index = virtualRow.index - const row = rows[index] - if (!row) return null - return ( - 0 ? pinnedOffsets : undefined} - lastPinnedColKey={lastPinnedColKey} - findMatchColumns={findMatchColumnsByRowId.get(row.id)} - /> - ) - })} - {paddingBottom > 0 && ( - - - - )} - {isFetchingNextPage && ( - - -
- -
- - - )} - - ) - })()} + {isLoadingTable || isLoadingRows ? null : rowsError ? ( + + + {rowsError.message} + + + ) : ( + (() => { + const virtualItems = rowVirtualizer.getVirtualItems() + // `item.start`/`item.end` include `scrollMargin` (the sticky-header + // offset) but `getTotalSize()` already nets it out, so both spacer + // heights are computed relative to `scrollMargin`. + const scrollMargin = rowVirtualizer.options.scrollMargin + const paddingTop = + virtualItems.length > 0 ? virtualItems[0].start - scrollMargin : 0 + const paddingBottom = + virtualItems.length > 0 + ? rowVirtualizer.getTotalSize() - + (virtualItems[virtualItems.length - 1].end - scrollMargin) + : 0 + return ( + <> + {paddingTop > 0 && ( + + + + )} + {virtualItems.map((virtualRow) => { + const index = virtualRow.index + const row = rows[index] + if (!row) return null + return ( + 0 ? pinnedOffsets : undefined} + lastPinnedColKey={lastPinnedColKey} + findMatchColumns={findMatchColumnsByRowId.get(row.id)} + /> + ) + })} + {paddingBottom > 0 && ( + + + + )} + {isFetchingNextPage && ( + + +
+ +
+ + + )} + + ) + })() + )} {remoteSelections.length > 0 && ( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts index f6333ae46f8..3b890d0c6ac 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts @@ -54,6 +54,12 @@ export interface UseTableReturn { */ filter: TablePredicate | null isLoadingRows: boolean + /** + * Why the first page of rows failed to load (e.g. a filter the server + * rejects), or null. Without it a rejected query is indistinguishable from + * an empty result. + */ + rowsError: Error | null refetchRows: () => void /** * The resolved value's `hasNextPage` reflects the post-fetch cache state — @@ -106,6 +112,7 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams) const { data: rowsData, isLoading: isLoadingRows, + error: rowsQueryError, refetch, fetchNextPage, hasNextPage, @@ -124,6 +131,10 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams) [rowsData?.pages] ) + // Only a failed first page leaves the grid with nothing to show; a failed + // next page or background refetch keeps the rows already loaded. + const rowsError = rowsData === undefined ? rowsQueryError : null + // Server-side COUNT(*) for the active filter (page 0 only). Null until the first page lands; // callers fall back to the table's unfiltered `rowCount`. This is the true "select all" total — // it reflects the filter, unlike `tableData.rowCount`. @@ -264,6 +275,7 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams) rowTotal, filter, isLoadingRows, + rowsError, refetchRows, fetchNextPage: fetchNextPageWrapped, hasNextPage: Boolean(hasNextPage), diff --git a/apps/sim/lib/table/import-runner.test.ts b/apps/sim/lib/table/import-runner.test.ts index a7e02987364..01cc977c239 100644 --- a/apps/sim/lib/table/import-runner.test.ts +++ b/apps/sim/lib/table/import-runner.test.ts @@ -1,6 +1,7 @@ import { Readable } from 'node:stream' import { posthogServerMock } from '@sim/testing/mocks/posthog-server.mock' import { storageServiceMock, storageServiceMockFns } from '@sim/testing/mocks/storage-service.mock' +import { tableBillingMock, tableBillingMockFns } from '@sim/testing/mocks/table-billing.mock' import { tableEventsMock } from '@sim/testing/mocks/table-events.mock' import { tableJobsServiceMock, @@ -18,6 +19,7 @@ const { mockBulkInsertImportBatch, mockNextImportStartPosition, mockNextImportSt })) vi.mock('@/lib/table/service', () => tableServiceMock) +vi.mock('@/lib/table/billing', () => tableBillingMock) vi.mock('@/lib/table/import-data', () => ({ addImportColumns: vi.fn(), bulkInsertImportBatch: mockBulkInsertImportBatch, @@ -45,6 +47,11 @@ const mockRecordImportRejections = tableJobsServiceMockFns.mockRecordImportRejec const mockDeleteFile = storageServiceMockFns.mockDeleteFile const mockDownloadFileStream = storageServiceMockFns.mockDownloadFileStream const mockHeadObject = storageServiceMockFns.mockHeadObject +const mockGetMaxRowsPerTable = tableBillingMockFns.mockGetMaxRowsPerTable + +beforeEach(() => { + mockGetMaxRowsPerTable.mockResolvedValue(-1) +}) const table = { id: 'tbl_1', @@ -283,3 +290,114 @@ describe('runTableImport in-flight progress', () => { expect(Math.max(...persisted)).toBe(CSV_SCHEMA_SAMPLE_SIZE) }) }) + +describe('runTableImport plan row limit', () => { + const limit = CSV_SCHEMA_SAMPLE_SIZE + 50 + + function csvOf(rowCount: number): string { + return `name\n${Array.from({ length: rowCount }, (_, i) => `row-${i}`).join('\n')}\n` + } + + function insertedRowCount(): number { + return mockBulkInsertImportBatch.mock.calls.reduce( + (total, [input]) => total + input.rows.length, + 0 + ) + } + + beforeEach(() => { + vi.clearAllMocks() + mockGetMaxRowsPerTable.mockResolvedValue(limit) + mockGetTableById.mockResolvedValue(table) + mockNextImportStartPosition.mockResolvedValue(0) + mockNextImportStartOrderKey.mockResolvedValue(null) + mockUpdateJobProgress.mockResolvedValue(true) + mockMarkJobReady.mockResolvedValue(true) + mockMarkJobFailed.mockResolvedValue(undefined) + mockDeleteFile.mockResolvedValue(undefined) + mockRecordImportRejections.mockResolvedValue(undefined) + mockBulkInsertImportBatch.mockImplementation(async ({ rows }: { rows: unknown[] }) => ({ + inserted: rows.length, + lastOrderKey: 'a1', + })) + }) + + function loadCsv(csv: string) { + mockHeadObject.mockResolvedValue({ size: Buffer.byteLength(csv) }) + mockDownloadFileStream.mockResolvedValue(Readable.from(csv)) + } + + it('fills the table to the limit when a batch crosses it, and reports the rows imported', async () => { + // The sample batch fits; the next batch crosses the limit. Rejecting that whole batch + // left the table short of its limit while the error claimed the limit was reached. + loadCsv(csvOf(limit * 2)) + + await runTableImport(buildPayload()) + + expect(insertedRowCount()).toBe(limit) + expect(mockMarkJobReady).not.toHaveBeenCalled() + expect(mockMarkJobFailed).toHaveBeenCalledWith( + 'tbl_1', + 'ws_1', + 'job_1', + expect.stringContaining(`${limit} rows were imported`) + ) + const persisted = mockUpdateJobProgress.mock.calls.map((call) => call[2]) + expect(Math.max(...persisted)).toBe(limit) + }) + + it('reports the rows imported when a batch lands exactly on the limit with rows left', async () => { + mockGetMaxRowsPerTable.mockResolvedValue(CSV_SCHEMA_SAMPLE_SIZE) + loadCsv(csvOf(CSV_SCHEMA_SAMPLE_SIZE + 25)) + + await runTableImport(buildPayload()) + + expect(insertedRowCount()).toBe(CSV_SCHEMA_SAMPLE_SIZE) + expect(mockMarkJobFailed).toHaveBeenCalledWith( + 'tbl_1', + 'ws_1', + 'job_1', + expect.stringContaining(`${CSV_SCHEMA_SAMPLE_SIZE} rows were imported`) + ) + }) + + it('counts existing rows against the limit on append', async () => { + mockGetTableById.mockResolvedValue({ ...table, rowCount: limit - 10 }) + loadCsv(csvOf(CSV_SCHEMA_SAMPLE_SIZE)) + + await runTableImport(buildPayload()) + + expect(insertedRowCount()).toBe(10) + expect(mockMarkJobFailed).toHaveBeenCalledWith( + 'tbl_1', + 'ws_1', + 'job_1', + expect.stringContaining('10 rows were imported') + ) + }) + + it('inserts nothing into a table that is already at its limit', async () => { + mockGetTableById.mockResolvedValue({ ...table, rowCount: limit }) + loadCsv(csvOf(5)) + + await runTableImport(buildPayload()) + + expect(mockBulkInsertImportBatch).not.toHaveBeenCalled() + expect(mockMarkJobFailed).toHaveBeenCalledWith( + 'tbl_1', + 'ws_1', + 'job_1', + expect.stringContaining('has reached its row limit') + ) + }) + + it('completes a file that exactly fills the table', async () => { + loadCsv(csvOf(limit)) + + await runTableImport(buildPayload()) + + expect(insertedRowCount()).toBe(limit) + expect(mockMarkJobFailed).not.toHaveBeenCalled() + expect(mockMarkJobReady).toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/table/import-runner.ts b/apps/sim/lib/table/import-runner.ts index 0185bc8ed60..235b12cfdf2 100644 --- a/apps/sim/lib/table/import-runner.ts +++ b/apps/sim/lib/table/import-runner.ts @@ -19,7 +19,7 @@ import { type TableSchema, validateMapping, } from '@/lib/table' -import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' +import { getMaxRowsPerTable, notifyTableRowUsage, TableRowLimitError } from '@/lib/table/billing' import { withGeneratedColumnIds } from '@/lib/table/column-keys' import { sniffCsvDelimiterFromStream } from '@/lib/table/csv-delimiter-stream' import { appendTableEvent } from '@/lib/table/events' @@ -56,6 +56,20 @@ const PROGRESS_INTERVAL_ROWS = 5000 */ class ImportSupersededError extends Error {} +/** + * Thrown once an import has filled the table to its plan row limit while the file still has + * rows left. The rows that fit stay committed (the runner never rolls back), so the message + * reports how many landed instead of claiming the table was already full. + */ +class ImportRowLimitReachedError extends Error { + constructor(limit: number, imported: number) { + super( + `Import stopped at this table's row limit (${limit.toLocaleString('en-US')} rows) on your current plan. ${imported.toLocaleString('en-US')} rows were imported; the rest of the file was not.` + ) + this.name = 'ImportRowLimitReachedError' + } +} + /** `create` infers a schema for a new table; `append`/`replace` map onto an existing one. */ export type TableImportMode = 'create' | 'append' | 'replace' @@ -182,7 +196,7 @@ export async function runTableImport(payload: TableImportPayload): Promise // Append keeps the existing rows; create/replace start from empty (replace deletes // existing rows in resolveSetup). Per-batch capacity is checked against this base + the - // running total, so a stream that crosses the plan limit fails within one batch. + // running total, so a stream that crosses the plan limit stops exactly at it. const existingRowCount = mode === 'append' ? table.rowCount : 0 // Count bytes as they flow so the row total can be extrapolated from byte progress. @@ -290,16 +304,28 @@ export async function runTableImport(payload: TableImportPayload): Promise if (mode === 'replace') await deleteAllTableRows(table, revalidateDelete) } - const flush = async (rows: Record[]) => { - if (rows.length === 0 || !schema || !headerToColumn) return + const flush = async (batchRows: Record[]) => { + if (batchRows.length === 0 || !schema || !headerToColumn) return // Ownership gate before every insert: once this run loses the table (cancel/supersede), // updateJobProgress returns false and we stop before writing into a table a newer import // may own. Runs per batch (not just at the emit cadence) so we stop within one batch. const owns = await updateJobProgressInWorkspace(tableId, workspaceId, inserted, importId) if (!owns) throw new ImportSupersededError() - // Held per batch and folded into the run total only once the rows commit: an - // assertRowCapacity rejection or a failed insert below discards this batch entirely, - // and counting its blanked cells would report loss for rows that never landed. + // Fill the table up to its plan limit rather than rejecting the whole batch that crosses + // it: batches are thousands of rows, so dropping the crossing batch strands the table + // well short of the limit it reports as reached. + const rowLimit = await getMaxRowsPerTable(workspaceId) + const capacity = + rowLimit < 0 ? batchRows.length : Math.max(0, rowLimit - (existingRowCount + inserted)) + if (capacity === 0) { + throw inserted > 0 + ? new ImportRowLimitReachedError(rowLimit, inserted) + : new TableRowLimitError(rowLimit) + } + const rows = capacity < batchRows.length ? batchRows.slice(0, capacity) : batchRows + // Held per batch and folded into the run total only once the rows commit: a failed insert + // below discards this batch entirely, and counting its blanked cells would report loss for + // rows that never landed. let batchCellsRejected = 0 const coerced = coerceRowsForTable( rows, @@ -310,11 +336,6 @@ export async function runTableImport(payload: TableImportPayload): Promise batchCellsRejected++ } ) - const rowLimit = await assertRowCapacity({ - workspaceId, - currentRowCount: existingRowCount + inserted, - addedRows: coerced.length, - }) const result = await bulkInsertImportBatch( { tableId, @@ -339,6 +360,10 @@ export async function runTableImport(payload: TableImportPayload): Promise inserted += result.inserted lastOrderKey = result.lastOrderKey cellsRejected += batchCellsRejected + if (rows.length < batchRows.length) { + await updateJobProgressInWorkspace(tableId, workspaceId, inserted, importId) + throw new ImportRowLimitReachedError(rowLimit, inserted) + } // Emit after the first batch, then every interval, so the bar appears early without flooding. if ( inserted - lastReported >= PROGRESS_INTERVAL_ROWS || From 5ff8eca71ba3804e98b3368e70577e7fc7e7c644 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 11 Nov 2026 10:33:00 -0800 Subject: [PATCH 2/2] fix(tables): use caption size for the grid error and drop the redundant mock reset --- .../tables/[tableId]/components/table-grid/table-grid.tsx | 2 +- apps/sim/lib/table/import-runner.test.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index a6bd0c37aa8..3ab57613818 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -4979,7 +4979,7 @@ export function TableGrid({ {rowsError.message} diff --git a/apps/sim/lib/table/import-runner.test.ts b/apps/sim/lib/table/import-runner.test.ts index 01cc977c239..fcc3adbfc0b 100644 --- a/apps/sim/lib/table/import-runner.test.ts +++ b/apps/sim/lib/table/import-runner.test.ts @@ -306,7 +306,6 @@ describe('runTableImport plan row limit', () => { } beforeEach(() => { - vi.clearAllMocks() mockGetMaxRowsPerTable.mockResolvedValue(limit) mockGetTableById.mockResolvedValue(table) mockNextImportStartPosition.mockResolvedValue(0)