fix(webapp): correct backward pagination slice in listRunIds#3867
Open
matt-aitken wants to merge 1 commit into
Open
fix(webapp): correct backward pagination slice in listRunIds#3867matt-aitken wants to merge 1 commit into
matt-aitken wants to merge 1 commit into
Conversation
Backward pagination dropped the wrong end of the result set. listRunRows fetches page.size + 1 rows to detect hasMore; that extra row is the one farthest from the cursor in both directions (forward orders DESC, backward orders ASC), so it is always the trailing element. Forward correctly sliced rows[0..size); backward+hasMore sliced rows[1..size+1], dropping the row closest to the cursor and keeping the has-more sentinel. The result was a page that straddled two logical pages (one run from the correct previous page plus one from the page before it) — so paging "newer" across a boundary repeated and skipped runs. Both directions now slice rows[0..size). Forward pagination, the hasMore=false backward path, and all cursor values were already correct and are unchanged. Adds a regression test that walks forward across multiple pages, then walks backward from the last page and asserts each backward page exactly reproduces the corresponding forward page (and the full traversal covers every run once). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🧰 Additional context used📓 Path-based instructions (11)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.server.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.{js,ts,tsx,jsx,css,json,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (25)📓 Common learnings📚 Learning: 2026-04-20T15:08:59.789ZApplied to files:
📚 Learning: 2025-07-12T18:06:04.133ZApplied to files:
📚 Learning: 2026-05-14T14:54:39.095ZApplied to files:
📚 Learning: 2026-04-16T14:19:16.330ZApplied to files:
📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-03-26T09:02:07.973ZApplied to files:
📚 Learning: 2026-05-05T09:38:02.512ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-06-04T18:16:35.386ZApplied to files:
📚 Learning: 2026-03-02T12:43:25.254ZApplied to files:
📚 Learning: 2026-04-16T13:45:22.317ZApplied to files:
📚 Learning: 2026-04-20T15:08:55.358ZApplied to files:
📚 Learning: 2026-04-13T21:44:00.032ZApplied to files:
📚 Learning: 2026-06-01T15:01:35.175ZApplied to files:
📚 Learning: 2026-04-07T14:12:18.946ZApplied to files:
📚 Learning: 2026-05-07T12:25:18.271ZApplied to files:
📚 Learning: 2026-03-03T13:07:33.177ZApplied to files:
📚 Learning: 2026-04-17T13:20:14.259ZApplied to files:
📚 Learning: 2026-06-02T21:20:56.997ZApplied to files:
📚 Learning: 2026-05-28T20:02:10.647ZApplied to files:
📚 Learning: 2026-05-18T14:40:02.173ZApplied to files:
🔇 Additional comments (3)
WalkthroughThis PR fixes an off-by-one bug in keyset pagination within 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🚩 Backward !hasMore edge case with partial pages produces null nextCursor
In the backward !hasMore branch (line 120), nextCursor = cursorFor(reversedRows.at(options.page.size - 1)). If the result has fewer rows than page.size (e.g., 1 row with page.size=2), reversedRows.at(1) is undefined → nextCursor becomes null. This means a user who backward-navigated to a partial first page cannot navigate forward from it. This is a pre-existing edge case (the cursor logic was not changed in this PR) and only affects degenerate scenarios where a backward navigation reaches the very first page with fewer items than page.size — in typical UI flows the forward pages were already visited so the user has other navigation paths.
(Refers to lines 119-121)
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ClickHouseRunsRepository.listRunIdsdropped the wrong end of the result set when paging backward.listRunRowsfetchespage.size + 1rows to detecthasMore. That extra row is the one farthest from the cursor in both directions (forward orderscreated_at DESC, run_id DESC; backward ordersASC), so it's always the trailing element. Forward correctly usedrows.slice(0, size), but backward+hasMoreused:— dropping the row closest to the cursor and keeping the has-more sentinel. The returned page straddled two logical pages: one run from the correct previous page plus one from the page before it. So paging "newer" (backward) across a page boundary repeated and skipped runs in the dashboard /
runs.list.This was pre-existing — it predates the composite-cursor work in #3852 and was spotted during it.
Fix
Both directions now slice
rows.slice(0, size)(the sentinel is the trailing element either way). Forward pagination, thehasMore === falsebackward path, and all cursor values were already correct and are unchanged — only the backward+hasMoreslice moves.Tests
New
runsRepositoryCursor.test.tscase: insert 5 runs (so a backward page has more rows before it →hasMore === true, the broken path), walk forward capturing every page, then walk backward from the last page and assert:mainwith{b,c}instead of{c,d}), andThe three existing cursor tests (forward completeness, backward round-trip, legacy cursor) still pass.
🤖 Generated with Claude Code