improvement(tables): harden pagination for short pages and surface name-validation errors

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.test.ts

@@ -79,7 +79,7 @@ function makeRow(id: string, position: number) {
   return { id, data: { name: `Row ${id}` }, position, executions: {} }
 }
 
-function makePages(rowsPerPage: number[], totalCount: number) {
+function makePages(rowsPerPage: number[], totalCount: number | null) {
   return rowsPerPage.map((count, pageIdx) => ({
     rows: Array.from({ length: count }, (_, i) =>
       makeRow(`r${pageIdx * 1000 + i}`, pageIdx * 1000 + i)
@@ -90,6 +90,10 @@ function makePages(rowsPerPage: number[], totalCount: number) {
 
 const OK = { status: 'success', hasNextPage: false } as const
 
+function makeHook(queryOptions = QUERY_OPTIONS) {
+  return useTable({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID, queryOptions })
+}
+
 beforeEach(() => {
   capturedEffects.length = 0
   vi.clearAllMocks()
@@ -100,83 +104,71 @@ beforeEach(() => {
 describe('useTable – ensureAllRowsLoaded', () => {
   it('returns an empty array when cache is empty', async () => {
     mockGetQueryData.mockReturnValue(undefined)
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+    const { ensureAllRowsLoaded } = makeHook()
     const rows = await ensureAllRowsLoaded()
     expect(rows).toEqual([])
     expect(mockFetchNextPage).not.toHaveBeenCalled()
   })
 
-  it('returns rows from cache immediately when last page is partial (< 1 page)', async () => {
-    const [page] = makePages([3], 3)
-    mockGetQueryData.mockReturnValue({ pages: [page] })
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+  it('returns cached rows without fetching when the count is covered by a partial page', async () => {
+    mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) })
+    const { ensureAllRowsLoaded } = makeHook()
     const rows = await ensureAllRowsLoaded()
     expect(rows).toHaveLength(3)
     expect(rows.map((r) => r.id)).toEqual(['r0', 'r1', 'r2'])
-    // Cache already complete — no HTTP request needed.
     expect(mockFetchNextPage).not.toHaveBeenCalled()
   })
 
-  it('returns rows from cache immediately when last page is exactly one full page', async () => {
-    // A full page means getNextPageParam returns the next offset, so we must
-    // fetch once to confirm there is no page 2 (which returns 0 rows). After
-    // that empty page the last page is partial (0 < 1000) and the loop breaks.
-    const [page0] = makePages([1000], 1000)
-    const emptyPage = { rows: [], totalCount: 1000 }
-    mockGetQueryData
-      .mockReturnValueOnce({ pages: [page0] }) // loop iter 1: full → fetch
-      .mockReturnValueOnce({ pages: [page0, emptyPage] }) // loop iter 2: empty → break
-      .mockReturnValue({ pages: [page0, emptyPage] }) // final read
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+  it('returns cached rows without fetching when the count is covered by exactly one full page', async () => {
+    // The totalCount fast-path terminates a covered drain without the
+    // empty-page confirmation request the old page-fullness heuristic needed.
+    mockGetQueryData.mockReturnValue({ pages: makePages([1000], 1000) })
+    const { ensureAllRowsLoaded } = makeHook()
     const rows = await ensureAllRowsLoaded()
     expect(rows).toHaveLength(1000)
-    expect(rows[0].id).toBe('r0')
-    expect(rows[999].id).toBe('r999')
+    expect(mockFetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('keeps paging past a short page when the count says more rows exist', async () => {
+    // The regression this termination rule exists for: a page shorter than the
+    // requested size must not be read as end-of-table.
+    const [shortPage] = makePages([36], 100)
+    const rest = {
+      rows: Array.from({ length: 64 }, (_, i) => makeRow(`r${1000 + i}`, 1000 + i)),
+      totalCount: null,
+    }
+    mockGetQueryData
+      .mockReturnValueOnce({ pages: [shortPage] }) // iter 1 check: 36 < 100 → fetch
+      .mockReturnValueOnce({ pages: [shortPage, rest] }) // iter 1 progress: 2 > 1
+      .mockReturnValue({ pages: [shortPage, rest] }) // iter 2 check: covered → break; final read
+    const { ensureAllRowsLoaded } = makeHook()
+    const rows = await ensureAllRowsLoaded()
+    expect(rows).toHaveLength(100)
     expect(mockFetchNextPage).toHaveBeenCalledTimes(1)
   })
 
-  it('fetches one page when last cached page is full and there is more data', async () => {
-    const [page0, page1] = makePages([1000, 500], 1500)
+  it('drains until an empty page when the count is unknown', async () => {
+    const [page0] = makePages([1000], null)
+    const emptyPage = { rows: [], totalCount: null }
     mockGetQueryData
-      .mockReturnValueOnce({ pages: [page0] }) // loop iter 1: full → fetch
-      .mockReturnValueOnce({ pages: [page0, page1] }) // loop iter 2: partial → break
-      .mockReturnValue({ pages: [page0, page1] }) // final read
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+      .mockReturnValueOnce({ pages: [page0] }) // iter 1 check: unknown count → fetch
+      .mockReturnValueOnce({ pages: [page0, emptyPage] }) // iter 1 progress: 2 > 1
+      .mockReturnValue({ pages: [page0, emptyPage] }) // iter 2 check: empty page → break; final read
+    const { ensureAllRowsLoaded } = makeHook()
     const rows = await ensureAllRowsLoaded()
-    expect(rows).toHaveLength(1500)
-    expect(rows[0].id).toBe('r0')
-    expect(rows[1000].id).toBe('r1000')
+    expect(rows).toHaveLength(1000)
     expect(mockFetchNextPage).toHaveBeenCalledTimes(1)
   })
 
-  it('fetches multiple pages for a large table until a partial page terminates the drain', async () => {
+  it('fetches multiple pages for a large table until the count is covered', async () => {
     const [page0, page1, page2] = makePages([1000, 1000, 500], 2500)
     mockGetQueryData
-      .mockReturnValueOnce({ pages: [page0] }) // iter 1: full → fetch
-      .mockReturnValueOnce({ pages: [page0, page1] }) // iter 2: full → fetch
-      .mockReturnValueOnce({ pages: [page0, page1, page2] }) // iter 3: partial → break
-      .mockReturnValue({ pages: [page0, page1, page2] }) // final read
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+      .mockReturnValueOnce({ pages: [page0] }) // iter 1 check: 1000 < 2500 → fetch
+      .mockReturnValueOnce({ pages: [page0, page1] }) // iter 1 progress: 2 > 1
+      .mockReturnValueOnce({ pages: [page0, page1] }) // iter 2 check: 2000 < 2500 → fetch
+      .mockReturnValueOnce({ pages: [page0, page1, page2] }) // iter 2 progress: 3 > 2
+      .mockReturnValue({ pages: [page0, page1, page2] }) // iter 3 check: covered → break; final read
+    const { ensureAllRowsLoaded } = makeHook()
     const rows = await ensureAllRowsLoaded()
     expect(rows).toHaveLength(2500)
     expect(rows[0].id).toBe('r0')
@@ -186,18 +178,21 @@ describe('useTable – ensureAllRowsLoaded', () => {
   })
 
   it('throws when fetchNextPage returns an error status', async () => {
-    const [page0] = makePages([1000], 2000)
-    mockGetQueryData.mockReturnValue({ pages: [page0] })
+    mockGetQueryData.mockReturnValue({ pages: makePages([1000], 2000) })
     const error = new Error('Network failure')
     mockFetchNextPage.mockResolvedValueOnce({ status: 'error', error })
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: QUERY_OPTIONS,
-    })
+    const { ensureAllRowsLoaded } = makeHook()
     await expect(ensureAllRowsLoaded()).rejects.toThrow('Network failure')
   })
 
+  it('throws when a fetch makes no progress instead of spinning', async () => {
+    // A cancelQueries race can resolve fetchNextPage without appending a page.
+    mockGetQueryData.mockReturnValue({ pages: makePages([1000], 2000) })
+    const { ensureAllRowsLoaded } = makeHook()
+    await expect(ensureAllRowsLoaded()).rejects.toThrow('no progress')
+    expect(mockFetchNextPage).toHaveBeenCalledTimes(1)
+  })
+
   it('does not call fetchNextPage or getQueryData when workspaceId is empty', async () => {
     const { ensureAllRowsLoaded } = useTable({
       workspaceId: '',
@@ -224,15 +219,63 @@ describe('useTable – ensureAllRowsLoaded', () => {
 
   it('encodes queryOptions.filter into the queryKey passed to getQueryData', async () => {
     const filter = { column: 'name', operator: 'eq', value: 'Alice' } as never
-    const [page] = makePages([3], 3)
-    mockGetQueryData.mockReturnValue({ pages: [page] })
-    const { ensureAllRowsLoaded } = useTable({
-      workspaceId: WORKSPACE_ID,
-      tableId: TABLE_ID,
-      queryOptions: { filter, sort: null },
-    })
+    mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) })
+    const { ensureAllRowsLoaded } = makeHook({ filter, sort: null })
     await ensureAllRowsLoaded()
     const queryKey = mockGetQueryData.mock.calls[0][0] as unknown[]
     expect(JSON.stringify(queryKey)).toContain('Alice')
   })
 })
+
+describe('useTable – ensureRowsLoadedUpTo', () => {
+  it('returns the first maxRows with hasMore when the cache already exceeds the cap', async () => {
+    mockGetQueryData.mockReturnValue({ pages: makePages([1000, 1000], 2000) })
+    const { ensureRowsLoadedUpTo } = makeHook()
+    const result = await ensureRowsLoadedUpTo(1500)
+    expect(result.rows).toHaveLength(1500)
+    expect(result.hasMore).toBe(true)
+    expect(mockFetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('returns everything with hasMore false when the table fits under the cap', async () => {
+    mockGetQueryData.mockReturnValue({ pages: makePages([3], 3) })
+    const { ensureRowsLoadedUpTo } = makeHook()
+    const result = await ensureRowsLoadedUpTo(50)
+    expect(result.rows).toHaveLength(3)
+    expect(result.hasMore).toBe(false)
+    expect(mockFetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('loads one row past the cap to make hasMore exact at the boundary', async () => {
+    const [page0, page1] = makePages([1000, 1000], 2000)
+    mockGetQueryData
+      .mockReturnValueOnce({ pages: [page0] }) // check: at cap but more exist → fetch
+      .mockReturnValueOnce({ pages: [page0, page1] }) // progress: 2 > 1
+      .mockReturnValue({ pages: [page0, page1] }) // check: past cap → break; final read
+    const { ensureRowsLoadedUpTo } = makeHook()
+    const result = await ensureRowsLoadedUpTo(1000)
+    expect(result.rows).toHaveLength(1000)
+    expect(result.hasMore).toBe(true)
+    expect(mockFetchNextPage).toHaveBeenCalledTimes(1)
+  })
+
+  it('skips the boundary probe when the count is already covered', async () => {
+    mockGetQueryData.mockReturnValue({ pages: makePages([1000], 1000) })
+    const { ensureRowsLoadedUpTo } = makeHook()
+    const result = await ensureRowsLoadedUpTo(1000)
+    expect(result.rows).toHaveLength(1000)
+    expect(result.hasMore).toBe(false)
+    expect(mockFetchNextPage).not.toHaveBeenCalled()
+  })
+
+  it('returns empty with hasMore false when ids are missing', async () => {
+    const { ensureRowsLoadedUpTo } = useTable({
+      workspaceId: '',
+      tableId: TABLE_ID,
+      queryOptions: QUERY_OPTIONS,
+    })
+    const result = await ensureRowsLoadedUpTo(10)
+    expect(result).toEqual({ rows: [], hasMore: false })
+    expect(mockFetchNextPage).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table.ts

@@ -11,6 +11,7 @@ import {
   useInfiniteTableRows,
   useTable as useTableQuery,
 } from '@/hooks/queries/tables'
+import { countLoadedTableRows, hasMoreTableRows } from '@/hooks/queries/utils/table-rows-pagination'
 import { useWorkflowStates, useWorkflows } from '@/hooks/queries/workflows'
 import type { WorkflowMetadata } from '@/stores/workflows/registry/types'
 import type { WorkflowState } from '@/stores/workflows/workflow/types'
@@ -124,13 +125,18 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams)
     // getQueryData bypasses React's render cycle — pages added by fetchNextPage
     // are visible synchronously after each await without waiting for a re-render.
     while (true) {
-      const data = queryClient.getQueryData(opts.queryKey)
-      const lastPage = data?.pages[data.pages.length - 1]
-      if (!lastPage || lastPage.rows.length < TABLE_LIMITS.MAX_QUERY_LIMIT) break
+      const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? []
+      if (!hasMoreTableRows(pages)) break
       const result = await fetchNextPage()
       if (result.status === 'error') {
         throw result.error ?? new Error('Failed to load table rows')
       }
+      const after = queryClient.getQueryData(opts.queryKey)?.pages.length ?? 0
+      if (after <= pages.length) {
+        // A cancelQueries race (optimistic mutation) can resolve the fetch without
+        // appending a page; retrying would spin forever on the same state.
+        throw new Error('Table rows pagination made no progress')
+      }
     }
 
     return queryClient.getQueryData(opts.queryKey)?.pages.flatMap((p) => p.rows) ?? []
@@ -148,25 +154,26 @@ export function useTable({ workspaceId, tableId, queryOptions }: UseTableParams)
         sort: queryOptions.sort,
       })
 
-      // Load one past the cap so `hasMore` is exact: a full final page only
-      // *might* have a successor, so we confirm by loading row `maxRows + 1`
-      // rather than inferring truncation from page fullness.
+      // Load one past the cap when needed so `hasMore` is exact: with the cap
+      // covered but hasMoreTableRows still true, row `maxRows + 1` confirms it.
       while (true) {
-        const data = queryClient.getQueryData(opts.queryKey)
-        const loaded = data?.pages.reduce((sum, p) => sum + p.rows.length, 0) ?? 0
-        if (loaded > maxRows) break
-        const lastPage = data?.pages[data.pages.length - 1]
-        if (!lastPage || lastPage.rows.length < TABLE_LIMITS.MAX_QUERY_LIMIT) break
+        const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? []
+        if (countLoadedTableRows(pages) > maxRows || !hasMoreTableRows(pages)) break
         const result = await fetchNextPage()
         if (result.status === 'error') {
           throw result.error ?? new Error('Failed to load table rows')
         }
+        const after = queryClient.getQueryData(opts.queryKey)?.pages.length ?? 0
+        if (after <= pages.length) {
+          throw new Error('Table rows pagination made no progress')
+        }
       }
 
-      const all = queryClient.getQueryData(opts.queryKey)?.pages.flatMap((p) => p.rows) ?? []
+      const pages = queryClient.getQueryData(opts.queryKey)?.pages ?? []
+      const all = pages.flatMap((p) => p.rows)
       return {
         rows: all.length > maxRows ? all.slice(0, maxRows) : all,
-        hasMore: all.length > maxRows,
+        hasMore: all.length > maxRows || hasMoreTableRows(pages),
       }
     },
     [workspaceId, tableId, queryOptions.filter, queryOptions.sort, queryClient, fetchNextPage]