fix(pagination): upgrade cursor encoding and SQL filtering for identi… - #1382
fix(pagination): upgrade cursor encoding and SQL filtering for identi…#1382simar2411 wants to merge 1 commit into
Conversation
…cal createdAt timestamps (knoxiboy#1335)
|
@Kritika200520 is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
@coderabbitai review |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe pagination cursor contract now supports comma-separated encoding, legacy pipe-separated cursors, and timestamp-only cursors. The doubts API applies composite or timestamp-only keyset boundaries based on the decoded cursor. ChangesCursor pagination
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
|
| const cursorRequested = searchParams.has("cursor"); | ||
| const decodedCursor = decodeCursor(searchParams.get("cursor")); | ||
| const cursorCreatedAt = decodedCursor?.createdAt ?? null; | ||
| const cursorCreatedAt = decodedCursor?.timestamp ?? decodedCursor?.createdAt ?? null; |
There was a problem hiding this comment.
Suggestion: Malformed non-empty cursors decode to null, making cursorCreatedAt null, but useCursor remains true because it depends only on the presence of the cursor parameter. The request therefore silently returns the first cursor-mode page and may issue a new cursor instead of falling back to offset pagination as documented. Require a valid decoded cursor before enabling cursor mode, or explicitly reject invalid cursors. [api mismatch]
Severity Level: Major ⚠️
- ⚠️ Malformed cursor requests select the wrong pagination mode.
- ❌ Pinned-first ordering is bypassed for affected requests.
- ⚠️ Clients may receive an unexpected replacement cursor.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/doubts/route.ts
**Line:** 158:158
**Comment:**
*Api Mismatch: Malformed non-empty cursors decode to `null`, making `cursorCreatedAt` null, but `useCursor` remains true because it depends only on the presence of the `cursor` parameter. The request therefore silently returns the first cursor-mode page and may issue a new cursor instead of falling back to offset pagination as documented. Require a valid decoded cursor before enabling cursor mode, or explicitly reject invalid cursors.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| lt(doubtsTable.id, cursorId), | ||
| ), | ||
| ) | ||
| : lt(doubtsTable.createdAt, cursorCreatedAt) |
There was a problem hiding this comment.
Suggestion: Legacy timestamp-only cursors produce cursorId === null, so this branch filters with only createdAt < cursorCreatedAt. Because cursor ordering is (createdAt DESC, id DESC), every row sharing the boundary timestamp is skipped, including rows that should follow the legacy cursor according to the id tiebreaker. Legacy cursors should either be migrated/rejected or handled with a defined inclusive boundary strategy. [off-by-one]
Severity Level: Major ⚠️
- ❌ Legacy pagination can omit doubts sharing timestamps.
- ⚠️ Users may never see skipped feed rows.
- ⚠️ Results differ from composite keyset ordering.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/doubts/route.ts
**Line:** 253:253
**Comment:**
*Off By One: Legacy timestamp-only cursors produce `cursorId === null`, so this branch filters with only `createdAt < cursorCreatedAt`. Because cursor ordering is `(createdAt DESC, id DESC)`, every row sharing the boundary timestamp is skipped, including rows that should follow the legacy cursor according to the id tiebreaker. Legacy cursors should either be migrated/rejected or handled with a defined inclusive boundary strategy.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/pagination.ts`:
- Around line 42-53: Update decodeCursor() to reject normalized invalid calendar
dates by requiring createdAt.toISOString() to exactly match the raw timestamp
string for both composite cursors using iso and legacy timestamp-only cursors
using decoded. Preserve the existing invalid-date, integer-id, and empty-id
validation, and add regression coverage for values such as
2026-02-30T00:00:00.270Z.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 46566d2b-28cb-4702-aa16-cf9d178a20aa
📒 Files selected for processing (3)
src/__tests__/lib/pagination.test.tssrc/app/api/doubts/route.tssrc/lib/pagination.ts
| const createdAt = new Date(iso); | ||
| const id = Number(idRaw); | ||
| if (Number.isNaN(createdAt.getTime()) || !Number.isInteger(id) || idRaw.trim() === "") { | ||
| return null; | ||
| } | ||
| return { createdAt, timestamp: createdAt, id }; | ||
| } | ||
|
|
||
| // Support legacy single-field timestamp cursors without an id component | ||
| const createdAt = new Date(decoded); | ||
| if (!Number.isNaN(createdAt.getTime())) { | ||
| return { createdAt, timestamp: createdAt, id: null }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify that invalid-calendar-date regression coverage exists after the fix.
rg -n -C 3 '2026-02-30|invalid date' src/__tests__/lib/pagination.test.tsRepository: knoxiboy/DoubtDesk
Length of output: 468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)pagination(\.test)?\.ts$|(^|/)route\.ts$|doubts' || true
echo
echo "pagination outline:"
ast-grep outline src/lib/pagination.ts --view expanded || true
echo
echo "pagination.ts lines 1-120:"
cat -n src/lib/pagination.ts | sed -n '1,120p'
echo
echo "pagination.test.ts relevant tests:"
cat -n src/__tests__/lib/pagination.test.ts | sed -n '1,140p'
echo
echo "cursor consumers:"
rg -n -C 4 'decodeCursor|timestamp|createdAt' src | sed -n '1,200p'
echo
echo "Behavioral probe for invalid calendar dates in available runtime:"
node - <<'JS'
const samples = [
"2026-02-30T00:00:00.270Z",
"2026-02-30T23:59:59.999Z",
"2026-01-32T00:00:00.000Z",
];
for (const s of samples) {
const d = new Date(s);
console.log(`${s} => ${d.getTime()} ISO=${d.toJSON()} timeString=${d.toString()}`);
}
JSRepository: knoxiboy/DoubtDesk
Length of output: 23410
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Cursor consumers around decodeCursor calls:"
rg -n -C 8 'decodeCursor\(' src | sed -n '1,240p'
echo
echo "Relevant pagination tests end:"
cat -n src/__tests__/lib/pagination.test.ts | sed -n '71,120p'Repository: knoxiboy/DoubtDesk
Length of output: 9435
Reject normalized invalid calendar dates.
decodeCursor() currently accepts "2026-02-30T00:00:00.000Z" because new Date() normalizes it to March 2, 2026. The route then uses that later boundary in the (createdAt, id) keyset predicate, which can skip valid rows. Reject timestamps whose toISOString() differs from the raw decoded date string for both composite and legacy timestamp-only cursors, and add regression cases like "2026-02-30T00:00:00.270Z".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/pagination.ts` around lines 42 - 53, Update decodeCursor() to reject
normalized invalid calendar dates by requiring createdAt.toISOString() to
exactly match the raw timestamp string for both composite cursors using iso and
legacy timestamp-only cursors using decoded. Preserve the existing invalid-date,
integer-id, and empty-id validation, and add regression coverage for values such
as 2026-02-30T00:00:00.270Z.
User description
…cal createdAt timestamps (#1335)
Description
Related Issue
Closes #
Type of Change
Screenshots (if UI change)
How Has This Been Tested?
npm run devChecklist
npm run dev)anytypes)mainCodeAnt-AI Description
Prevent cursor pagination from skipping items with identical creation times
What Changed
Impact
✅ Fewer skipped items across paginated feeds✅ Reliable pagination for identical creation timestamps✅ Safer handling of legacy and invalid cursors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit