[adityareddy-dev]

Summary

A short search page triggered another query in the same time window. I changed pagination to move to the next window, or stop after the last one.

Fixes #1403.

How to reproduce

Starting point: A search spanning multiple time windows, with a page limit of 100 and only one matching row in the first window.

On main:

1. Fetch the first page, then request the next page.
2. The next query repeats the first window with OFFSET 1.

On this branch:

1. Repeat the same search.
2. The next query starts the second window with OFFSET 0.

How to test on Vercel preview

N/A, query pagination change with no visual changes.

References

#1403

[github-actions]

Hi @adityareddy-dev, thanks for the pull request!

Before we review code from a first-time contributor, a maintainer vouches for them, and you're not on our list yet. There's nothing else to file and this PR stays open — a maintainer will look at it, usually within a day or two, and vouch for you right here if it's a genuine attempt at the problem.

To make that quick, check the PR description has:

• The issue it addresses (ideally one labelled Accepting PR) and why the change is needed
• Steps to reproduce the problem and verify the fix, starting from https://play.hyperdx.io or yarn app:dev:local connected to the demo server
• Before/after screenshots if anything in the UI changed

More detail in our contributing guide.

[github-actions]

PR Review

✅ No issues found.

[github-actions]

Deep Review

Report-only multi-agent review of the pagination change in useOffsetPaginatedQuery. The diff makes getNextPageParam advance to the next time window when the last page is shorter than the requested limit (or when there is no limit), instead of issuing a redundant same-window OFFSET query. The core fix is sound: a SQL LIMIT query returning fewer rows than the limit genuinely means the window is exhausted, and the new tests assert the redundant probe query is gone (they count renderChartConfig/query calls, not just rows). No critical issues found.

🟡 P2 — recommended

• packages/app/src/hooks/useOffsetPaginatedQuery.tsx:154 — The fix equates "page shorter than the limit" with "window exhausted", but a builder query whose result is truncated by a row cap (max_result_rows + result_overflow_mode=break arriving via source.querySettings or a server profile) returns fewer than limit rows while more matching rows remain, so the new branch skips to the next window and silently drops them; the previous data.length > 0 branch would have kept offset-paging and drained the window.
  • Fix: Document the short-page-means-exhausted invariant at the branch and only treat a short page as exhausted when no result-truncating setting can apply to the query, otherwise continue offset pagination within the window.
  • _{adversarial, correctness, maintainability}

🔵 P3 nitpicks (3)

• packages/app/src/hooks/useOffsetPaginatedQuery.tsx:153 — getNextPageParam derives windowing, window count, and now the limit threshold from the raw closure config, while queryFn executes optimizedConfig ?? rawConfig; this is safe today only because MV optimization preserves limit and orderBy (it rewrites timestampValueExpression, from, select, and dateRange), so the coupling is a latent drift hazard the new config.limit dependency now rides on.
  • Fix: Pass the same effective config queryFn uses (optimizedConfig ?? rawConfig) into getNextPageParam so the windowing, order, window-count, and limit decisions all match the executed query.
• packages/app/src/hooks/useOffsetPaginatedQuery.tsx:175 — The isBuilderChartConfig(config) && isTimestampExpressionInFirstOrderBy(config) predicate is computed identically here and in queryFn, and the window array is built with a different guard than getTimeWindowFromPageParam uses.
  • Fix: Hoist a shared shouldUseWindowing(config) helper and a shared buildTimeWindows(config, …) helper so the duplicated predicates and window generation cannot drift.
• packages/app/src/hooks/__tests__/useOffsetPaginatedQuery.test.tsx:571 — Deep within-window pagination (3+ cumulative full pages) and the advance-on-short-page transition are each covered separately but never as one scenario.
  • Fix: Add an it.each case where a non-final window returns several full pages (offsets 0, 2, 4, …) then a short page, asserting the offsets accumulate and the final call starts the next window at offset 0.

---

Reviewers (10): correctness, adversarial, testing, maintainability, project-standards, kieran-typescript, performance, previous-comments, agent-native, learnings-researcher.

Testing gaps:

• No test exercises a builder page truncated by a server/source row cap (max_result_rows + result_overflow_mode=break) rather than genuine exhaustion.
• No test covers divergence between the optimized and raw config (rewritten timestampValueExpression/windowing) on the paginated path.
• No test covers limit: 0 through getNextPageParam.

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[adityareddy-dev]

Done.