Skip to content

test(auto-paginate): fix over-strict cursor contract at exact page boundary#1060

Merged
BYK merged 1 commit into
mainfrom
fix/autopaginate-exact-boundary-cursor-test
Jun 3, 2026
Merged

test(auto-paginate): fix over-strict cursor contract at exact page boundary#1060
BYK merged 1 commit into
mainfrom
fix/autopaginate-exact-boundary-cursor-test

Conversation

@BYK
Copy link
Copy Markdown
Member

@BYK BYK commented Jun 3, 2026

Summary

Fixes a flaky property test, auto-paginate.property.test.ts
"drops nextCursor whenever the result is trimmed to limit", which failed on
seeds that hit an exact page-boundary input (e.g. counterexample
[total=393, limit=392, pageSize=98], since 392 % 98 === 0).

This is a test-only fix — the autoPaginate() implementation is correct.

Root cause

The test asserted that whenever total > limit, nextCursor must be
undefined. That predicate conflates two cases with opposite, correct
contracts in the multi-page path:

  • True overshoot — a page pushes accumulation strictly past limit. Rows are
    trimmed and the server cursor points past the trimmed tail, so it is dropped.
    Preserving it would skip rows (the original skip-bug guard). ✅
  • Exact boundary (limit % effectivePageSize === 0) — accumulation lands
    exactly on limit, nothing is trimmed, and the last page's cursor points at
    index limit (the first unreturned row). It must be preserved so callers
    can page forward; dropping it strands the tail — the regression explicitly
    pinned by events-overshoot.test.ts. ✅

In the counterexample, autoPaginate correctly returns nextCursor="392"
(following it yields row 392 — contiguous, no skip), but the test demanded
undefined.

Fix

Split the assertion by the page-boundary discriminator:

  • limit % effectivePageSize === 0 (exact boundary) → assert
    nextCursor === String(limit) (positively verifies the cursor resumes
    contiguously).
  • otherwise (true overshoot) → assert nextCursor === undefined (skip-bug
    guard, unchanged).

Also updated the file header to document the boundary nuance.

Why not change the implementation?

Dropping the cursor at the exact boundary would reintroduce the stranding
regression that events-overshoot.test.ts guards against (-c next could never
advance past limit). The implementation's two-branch logic is correct.

Verification

  • Exhaustive sweep of all (total, limit, pageSize) combinations in the
    test's arbitrary ranges (103,171 cases): 0 violations against the new
    contract.
  • auto-paginate.property.test.ts + events-overshoot.test.ts: pass.
  • Property test run 15× consecutively: 15/15 green (was intermittently
    failing).
  • pnpm run lint: clean. pnpm exec tsc --noEmit: exit 0.

Context

This flake was spotted during the full-suite run while fixing the response-cache
torn-read bug (#1056) and noted there as a follow-up.

…undary

The "drops nextCursor whenever the result is trimmed to limit" property test
was flaky (~per-seed): it asserted that `total > limit` always implies
`nextCursor === undefined`. That conflates two cases with opposite, correct
contracts in the multi-page path of autoPaginate():

- True overshoot (a page pushes accumulation strictly past `limit`): rows are
  trimmed and the server cursor points past the trimmed tail, so it is dropped.
  Preserving it would skip rows — the original skip-bug guard.
- Exact boundary (`limit % effectivePageSize === 0`): accumulation lands exactly
  on `limit`, nothing is trimmed, and the last page's cursor points at index
  `limit` — the first unreturned row. It MUST be preserved so callers can page
  forward; dropping it strands the tail (the regression pinned by
  events-overshoot.test.ts).

The counterexample [total=393, limit=392, pageSize=98] is the exact-boundary
case (392 % 98 === 0): the implementation correctly returns nextCursor="392",
but the test demanded undefined.

This is a test-only fix — the implementation is correct. Verified by an
exhaustive sweep of all (total, limit, pageSize) combinations in the test's
arbitrary ranges: 0 violations against the new contract.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Codecov Results 📊

✅ Patch coverage is 100.00%. Project has 4420 uncovered lines.
✅ Project coverage is 81.85%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.85%    81.85%        —%
==========================================
  Files          341       341         —
  Lines        24358     24358         —
  Branches     15912     15912         —
==========================================
+ Hits         19937     19938        +1
- Misses        4421      4420        -1
- Partials      1665      1664        -1

Generated by Codecov Action

@BYK BYK merged commit df9dfae into main Jun 3, 2026
26 checks passed
@BYK BYK deleted the fix/autopaginate-exact-boundary-cursor-test branch June 3, 2026 14:48
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