Skip to content

Commit 431e177

Browse files
fix(tables): align getNextPageParam tests with count-based termination
1 parent 1669b2f commit 431e177

3 files changed

Lines changed: 66 additions & 28 deletions

File tree

apps/sim/hooks/queries/tables.test.ts

Lines changed: 56 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,11 @@ describe('tableRowsParamsKey', () => {
348348
describe('tableRowsInfiniteOptions', () => {
349349
const PAGE_SIZE = 1000
350350

351+
interface PageFixture {
352+
rows: Array<{ id: string; orderKey?: string }>
353+
totalCount: number | null
354+
}
355+
351356
function makeOpts(pageSize = PAGE_SIZE, sort: unknown = null) {
352357
return tableRowsInfiniteOptions({
353358
workspaceId: WORKSPACE_ID,
@@ -358,59 +363,84 @@ describe('tableRowsInfiniteOptions', () => {
358363
}) as {
359364
queryKey: readonly unknown[]
360365
getNextPageParam: (
361-
lastPage: { rows: unknown[] },
362-
allPages: unknown[],
366+
lastPage: PageFixture,
367+
allPages: PageFixture[],
363368
lastPageParam: unknown
364369
) => number | { orderKey: string; id: string } | undefined
365370
}
366371
}
367372

368-
it('getNextPageParam returns undefined for a partial page (drain terminates)', () => {
373+
function makePage(count: number, totalCount: number | null, startAt = 0, withOrderKey = false) {
374+
return {
375+
rows: Array.from({ length: count }, (_, i) => ({
376+
id: `r${startAt + i}`,
377+
...(withOrderKey ? { orderKey: `a${startAt + i}` } : {}),
378+
})),
379+
totalCount,
380+
}
381+
}
382+
383+
function next(
384+
opts: ReturnType<typeof makeOpts>,
385+
pages: PageFixture[],
386+
lastPageParam: unknown = 0
387+
) {
388+
return opts.getNextPageParam(pages[pages.length - 1], pages, lastPageParam)
389+
}
390+
391+
it('getNextPageParam terminates when the count is covered by a partial page', () => {
369392
const opts = makeOpts()
370-
const lastPage = { rows: Array.from({ length: 500 }, (_, i) => ({ id: `r${i}` })) }
371-
expect(opts.getNextPageParam(lastPage, [], 0)).toBeUndefined()
393+
expect(next(opts, [makePage(500, 500)])).toBeUndefined()
372394
})
373395

374-
it('getNextPageParam returns undefined for an empty page', () => {
396+
it('getNextPageParam terminates on an empty page', () => {
375397
const opts = makeOpts()
376-
expect(opts.getNextPageParam({ rows: [] }, [], 0)).toBeUndefined()
398+
expect(next(opts, [makePage(1000, null), makePage(0, null, 1000)])).toBeUndefined()
377399
})
378400

379-
it('getNextPageParam returns next offset for a full page', () => {
401+
it('getNextPageParam continues past a short page when the count says more rows exist', () => {
402+
// The regression the termination rule exists for: a page shorter than the
403+
// requested size (e.g. a byte-cut page) must not be read as end-of-table.
380404
const opts = makeOpts()
381-
const fullPage = { rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}` })) }
382-
expect(opts.getNextPageParam(fullPage, [], 0)).toBe(PAGE_SIZE)
383-
expect(opts.getNextPageParam(fullPage, [], PAGE_SIZE)).toBe(PAGE_SIZE * 2)
405+
expect(next(opts, [makePage(36, 100)])).toBe(36)
384406
})
385407

386-
it('getNextPageParam advances correctly across three pages of 1000', () => {
408+
it('getNextPageParam terminates a full page when the count is covered', () => {
387409
const opts = makeOpts()
388-
const fullPage = { rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}` })) }
389-
const lastPartialPage = { rows: Array.from({ length: 200 }, (_, i) => ({ id: `r${i}` })) }
410+
expect(next(opts, [makePage(PAGE_SIZE, PAGE_SIZE)])).toBeUndefined()
411+
})
390412

391-
expect(opts.getNextPageParam(fullPage, [], 0)).toBe(1000)
392-
expect(opts.getNextPageParam(fullPage, [], 1000)).toBe(2000)
393-
expect(opts.getNextPageParam(lastPartialPage, [], 2000)).toBeUndefined()
413+
it('getNextPageParam returns next offset for a full page with an unknown count', () => {
414+
const opts = makeOpts()
415+
expect(next(opts, [makePage(PAGE_SIZE, null)])).toBe(PAGE_SIZE)
416+
})
417+
418+
it('getNextPageParam advances correctly across three pages', () => {
419+
const opts = makeOpts()
420+
const p0 = makePage(PAGE_SIZE, 2200)
421+
const p1 = makePage(PAGE_SIZE, null, 1000)
422+
const p2 = makePage(200, null, 2000)
423+
424+
expect(next(opts, [p0])).toBe(1000)
425+
expect(next(opts, [p0, p1], 1000)).toBe(2000)
426+
expect(next(opts, [p0, p1, p2], 2000)).toBeUndefined()
394427
})
395428

396429
it('getNextPageParam returns a keyset cursor when rows carry orderKey and there is no sort', () => {
397430
const opts = makeOpts()
398-
const fullPage = {
399-
rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}`, orderKey: `a${i}` })),
400-
}
401-
expect(opts.getNextPageParam(fullPage, [], 0)).toEqual({
431+
const pages = [makePage(PAGE_SIZE, 2000, 0, true)]
432+
expect(next(opts, pages)).toEqual({
402433
orderKey: `a${PAGE_SIZE - 1}`,
403434
id: `r${PAGE_SIZE - 1}`,
404435
})
405436
})
406437

407438
it('getNextPageParam falls back to offset for sorted views even with orderKey present', () => {
408439
const opts = makeOpts(PAGE_SIZE, { column: 'name', direction: 'asc' })
409-
const fullPage = {
410-
rows: Array.from({ length: PAGE_SIZE }, (_, i) => ({ id: `r${i}`, orderKey: `a${i}` })),
411-
}
412-
expect(opts.getNextPageParam(fullPage, [], 0)).toBe(PAGE_SIZE)
413-
expect(opts.getNextPageParam(fullPage, [], PAGE_SIZE)).toBe(PAGE_SIZE * 2)
440+
const p0 = makePage(PAGE_SIZE, 3000, 0, true)
441+
const p1 = makePage(PAGE_SIZE, null, 1000, true)
442+
expect(next(opts, [p0])).toBe(PAGE_SIZE)
443+
expect(next(opts, [p0, p1], PAGE_SIZE)).toBe(PAGE_SIZE * 2)
414444
})
415445

416446
it('queryKey includes the result of tableRowsParamsKey', () => {

apps/sim/hooks/queries/utils/table-rows-pagination.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@ export function countLoadedTableRows(pages: readonly TableRowsPageLike[]): numbe
2020
* Whether more rows may exist past the fetched pages. A page is terminal only when it is
2121
* empty or when page 0's `COUNT(*)` is already covered — never when it is merely shorter
2222
* than the requested page size, so a short server page can never be misread as end-of-table.
23-
* `totalCount` is advisory (computed in a separate transaction from the page read); the
24-
* empty-page rule is the correctness backstop, costing at most one extra request.
23+
*
24+
* `totalCount` is advisory (computed in a separate transaction from the page read). A
25+
* stale-high count self-corrects via the empty-page rule at the cost of one extra request;
26+
* a stale-low count (rows deleted after page 0's COUNT) stops the drain early — accepted,
27+
* since the view is already stale and the run-stream/interval invalidations refetch it.
2528
*/
2629
export function hasMoreTableRows(pages: readonly TableRowsPageLike[]): boolean {
2730
const lastPage = pages[pages.length - 1]

apps/sim/lib/table/rows/paging.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22
* Cuts a fetched page to a byte budget: keeps the longest prefix of rows whose
33
* serialized `data` fits within `maxBytes`, always keeping at least one row so
44
* pagination makes forward progress even when a single row exceeds the budget.
5+
*
6+
* The budget counts `data` only — the per-row envelope (`id`, `position`,
7+
* `orderKey`, timestamps, executions) is not measured, so actual response
8+
* payloads run slightly over `maxBytes`. Callers must leave headroom; the
9+
* production SQL-side cut should account for the same overhead.
510
*/
611
export function trimRowsToByteBudget<T extends { data: unknown }>(
712
rows: T[],

0 commit comments

Comments
 (0)