fix(replies): add cursor pagination for doubt replies - #1076
fix(replies): add cursor pagination for doubt replies#1076Shreya-nipunge wants to merge 5 commits into
Conversation
|
@Shreya-nipunge 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 · |
github-actions
Bot
added
gssoc'26
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe replies GET endpoint now supports cursor-based pagination and returns pagination metadata. Classroom views retrieve successive pages, while API tests update mocks and assertions for the nested response shape and page-scoped upvote state. ChangesReply pagination
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ClassroomView
participant RepliesAPI
participant RepliesDatabase
ClassroomView->>RepliesAPI: Request replies with doubtId, limit, and cursor
RepliesAPI->>RepliesDatabase: Query ordered page with limit + 1
RepliesDatabase-->>RepliesAPI: Reply rows and extra-row indicator
RepliesAPI-->>ClassroomView: replies, nextCursor, and hasMore
ClassroomView->>RepliesAPI: Request next page while hasMore is true
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/classroom/AskAIView.tsx (1)
91-146: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNo staleness guard if
initialDoubtchanges mid-fetch.The
do...whileloop can now span several sequential requests (up to N pages) before resolving. IfinitialDoubtchanges while a previous run is still in-flight, the stale run'ssetMessages([initialUserMsg, assistantMsg])can resolve after the newer effect started and overwrite the current conversation with an answer for the wrong doubt. There's no cleanup function returned from thisuseEffectto guard against it.♻️ Suggested guard
useEffect(() => { if (initialDoubt) { + let cancelled = false; ... const fetchSolution = async () => { setIsLoading(true); try { ... do { ... } while (!solution && hasMore); - if (solution) { + if (solution && !cancelled) { ... setMessages([initialUserMsg, assistantMsg]); } } catch (err) { console.error("Error fetching solution for initial doubt:", err); } finally { - setIsLoading(false); + if (!cancelled) setIsLoading(false); } }; void fetchSolution(); + return () => { cancelled = true; }; } }, [initialDoubt]);🤖 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/components/classroom/AskAIView.tsx` around lines 91 - 146, Protect the fetchSolution effect from stale asynchronous results when initialDoubt changes. Add an effect-scoped cancellation or active flag with cleanup, check it before applying fetched messages and loading state, and ensure the cleanup invalidates prior runs so only the latest initialDoubt can call setMessages.src/components/classroom/DoubtRepliesModal.tsx (1)
120-154: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winFetch failures mid-pagination are silent — no error state, and partial results are discarded.
If any page request in the loop fails (
!res.okor JSON parse error), the functionreturns immediately: no toast/error UI is shown (unlikehandlePost,handleVote, etc. elsewhere in this component, which do surfacetoast.error), and any replies already accumulated from earlier successful pages inallRepliesare dropped instead of being shown viasetReplies(allReplies). Users get an indefinite/incomplete thread with no indication anything went wrong.♻️ Suggested fix
const res = await fetch(`/api/replies?${params}`); if (!res.ok) { console.error(`Replies API failed with status ${res.status}`); - return; + toast.error("Failed to load replies.", { id: `replies-fetch-error-${doubt.id}` }); + break; } let json; try { json = await res.json(); } catch (err) { console.error("Failed to parse replies response:", err); - return; + toast.error("Failed to load replies.", { id: `replies-fetch-error-${doubt.id}` }); + break; } allReplies = allReplies.concat(json.replies); cursor = json.nextCursor; hasMore = json.hasMore; } while (hasMore); setReplies(allReplies);As per path instructions, "Missing loading/error states" should be reviewed for
.tsxfiles.🤖 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/components/classroom/DoubtRepliesModal.tsx` around lines 120 - 154, Update fetchReplies to surface pagination failures through the component’s existing toast.error pattern and preserve already fetched replies by calling setReplies(allReplies) before exiting on non-OK responses or JSON parse errors. Ensure the loading state still clears through the existing finally block and avoid silently discarding partial results.Source: Path instructions
🧹 Nitpick comments (3)
src/components/classroom/AskAIView.tsx (1)
117-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded string comparisons for solution detection.
"solution"and"DoubtDesk AI"are inline string literals used to identify AI-generated replies; consider extracting them to shared constants to avoid silent drift if the replytypeenum or AI display name changes elsewhere.As per path instructions, "No hardcoded strings (use constants)" for
.tsxfiles.🤖 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/components/classroom/AskAIView.tsx` around lines 117 - 121, Replace the inline "solution" and "DoubtDesk AI" comparisons in the reply lookup within AskAIView with shared constants, reusing existing definitions if available or introducing appropriately named constants in the shared reply configuration. Keep the solution-detection behavior unchanged while ensuring future enum or display-name changes use a single source of truth.Source: Path instructions
src/__tests__/api/replies.test.ts (1)
86-87: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a tie-break ordering test.
These fixtures use distinct
createdAtvalues, so they wouldn't surface the missingidtiebreaker in the route'sorderBy(flagged separately insrc/app/api/replies/route.ts). Consider adding a case with two replies sharing an identicalcreatedAtand asserting the query builder was called with bothasc(createdAt)andasc(id), or at least a stable ordering of the returned page.Also applies to: 111-111, 123-131
🤖 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/__tests__/api/replies.test.ts` around lines 86 - 87, Extend the replies tests around the existing fixtures to include two replies with identical createdAt values, then verify stable tie-break ordering by asserting the query builder uses both asc(createdAt) and asc(id), or by asserting the returned page is ordered consistently by id. Update the related cases at the referenced test sections while preserving existing distinct-timestamp coverage.src/components/classroom/DoubtRepliesModal.tsx (1)
122-148: 🚀 Performance & Scalability | 🔵 TrivialConsider incremental loading rather than eagerly fetching every page.
fetchReplies(andAskAIView's solution search) now loops through every page before returning results, so for very active threads the client still accumulates the entire reply history in memory and the initial render is blocked until all pages complete — the per-request payload is bounded, but the round-trip count and end-to-end load time still scale with thread size. Since the linked issue calls out "Load More"/infinite scroll as the intended UX, worth considering surfacing the first page immediately and fetching subsequent pages on demand or in the background, if this hasn't been deferred intentionally for a later iteration.🤖 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/components/classroom/DoubtRepliesModal.tsx` around lines 122 - 148, Update fetchReplies in DoubtRepliesModal to expose the first replies page immediately instead of awaiting every pagination request before setReplies. Retain the nextCursor/hasMore state and fetch subsequent pages through an on-demand or background load-more flow, appending results to the existing replies without duplicating requests.
🤖 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/app/api/replies/route.ts`:
- Around line 105-117: The replies query’s ordering does not match its
`(createdAt, id)` keyset cursor. Update the `orderBy` in the replies fetch to
sort ascending by `repliesTable.createdAt` and then `repliesTable.id`,
preserving the existing limit and pagination logic.
---
Outside diff comments:
In `@src/components/classroom/AskAIView.tsx`:
- Around line 91-146: Protect the fetchSolution effect from stale asynchronous
results when initialDoubt changes. Add an effect-scoped cancellation or active
flag with cleanup, check it before applying fetched messages and loading state,
and ensure the cleanup invalidates prior runs so only the latest initialDoubt
can call setMessages.
In `@src/components/classroom/DoubtRepliesModal.tsx`:
- Around line 120-154: Update fetchReplies to surface pagination failures
through the component’s existing toast.error pattern and preserve already
fetched replies by calling setReplies(allReplies) before exiting on non-OK
responses or JSON parse errors. Ensure the loading state still clears through
the existing finally block and avoid silently discarding partial results.
---
Nitpick comments:
In `@src/__tests__/api/replies.test.ts`:
- Around line 86-87: Extend the replies tests around the existing fixtures to
include two replies with identical createdAt values, then verify stable
tie-break ordering by asserting the query builder uses both asc(createdAt) and
asc(id), or by asserting the returned page is ordered consistently by id. Update
the related cases at the referenced test sections while preserving existing
distinct-timestamp coverage.
In `@src/components/classroom/AskAIView.tsx`:
- Around line 117-121: Replace the inline "solution" and "DoubtDesk AI"
comparisons in the reply lookup within AskAIView with shared constants, reusing
existing definitions if available or introducing appropriately named constants
in the shared reply configuration. Keep the solution-detection behavior
unchanged while ensuring future enum or display-name changes use a single source
of truth.
In `@src/components/classroom/DoubtRepliesModal.tsx`:
- Around line 122-148: Update fetchReplies in DoubtRepliesModal to expose the
first replies page immediately instead of awaiting every pagination request
before setReplies. Retain the nextCursor/hasMore state and fetch subsequent
pages through an on-demand or background load-more flow, appending results to
the existing replies without duplicating requests.
🪄 Autofix (Beta)
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: 66c69207-91f6-4715-977c-5cb44325b257
📒 Files selected for processing (4)
src/__tests__/api/replies.test.tssrc/app/api/replies/route.tssrc/components/classroom/AskAIView.tsxsrc/components/classroom/DoubtRepliesModal.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/classroom/DoubtRepliesModal.tsx (2)
122-148: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winDeduplicate the full pagination load.
fetchRepliesis invoked by both effects at Lines 69 and 84. With this loop fetching every page, one modal open can issue the entire page sequence twice and race twosetRepliesupdates. Consolidate the triggers or deduplicate/abort an in-flight request before adding pagination.🤖 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/components/classroom/DoubtRepliesModal.tsx` around lines 122 - 148, Prevent duplicate full-pagination loads from fetchReplies being triggered by both effects; consolidate the effect triggers or cancel/reuse the existing in-flight request before starting another. Ensure one modal open performs a single pagination sequence and only its active request updates setReplies.
134-150: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not commit partial pagination results as a successful load.
When a later page fails,
breakis followed bysetReplies(allReplies), so the modal displays an incomplete thread and count while only showing a transient toast. Retain the previous data or expose a persistent retry/error state, and commit the accumulated replies only after all pages load successfully.As per path instructions, this TSX flow must provide a reliable loading/error state.
🤖 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/components/classroom/DoubtRepliesModal.tsx` around lines 134 - 150, Update the pagination flow in DoubtRepliesModal so failures from later requests or response parsing do not fall through to setReplies(allReplies). Track the load as unsuccessful, preserve the existing replies (or expose a persistent retry/error state), and call setReplies only after every page completes successfully; ensure the loading/error state remains reliable.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/components/classroom/DoubtRepliesModal.tsx`:
- Around line 122-148: Prevent duplicate full-pagination loads from fetchReplies
being triggered by both effects; consolidate the effect triggers or cancel/reuse
the existing in-flight request before starting another. Ensure one modal open
performs a single pagination sequence and only its active request updates
setReplies.
- Around line 134-150: Update the pagination flow in DoubtRepliesModal so
failures from later requests or response parsing do not fall through to
setReplies(allReplies). Track the load as unsuccessful, preserve the existing
replies (or expose a persistent retry/error state), and call setReplies only
after every page completes successfully; ensure the loading/error state remains
reliable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5cead9ba-adca-4709-a61c-9d1e772d3a11
📒 Files selected for processing (3)
src/app/api/replies/route.tssrc/components/classroom/AskAIView.tsxsrc/components/classroom/DoubtRepliesModal.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/classroom/AskAIView.tsx
- src/app/api/replies/route.ts
knoxiboy
added
level:intermediate
|
Hi @Shreya-nipunge! Thanks for implementing cursor-based pagination for doubt replies (#1062). The backend cursor encoding/decoding in Problem BreakdownIn let allReplies: Reply[] = [];
let cursor: string | null = null;
let hasMore = false;
do {
const params = new URLSearchParams({ doubtId: String(doubt.id) });
params.set("limit", "100");
if (cursor) params.set("cursor", cursor);
const res = await fetch(`/api/replies?${params}`);
...
allReplies = allReplies.concat(json.replies);
cursor = json.nextCursor;
hasMore = json.hasMore;
} while (hasMore);Why this needs adjustment:
Suggested FixIn Thanks for your hard work on this optimization! |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
github-actions
Bot
added
size/l
and removed
size/m
size:L
|
@knoxiboy ,Thanks for the feedback! I've updated the frontend pagination flow as suggested. The modal now fetches only the first page on open (using the default page size) and stores This preserves the benefits of pagination by avoiding eager requests and unnecessary memory usage while keeping the existing reply functionality unchanged. I've also verified that the changes pass TypeScript and ESLint checks. |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/components/classroom/DoubtRepliesModal.tsx`:
- Around line 180-182: Update the reply merge in DoubtRepliesModal so newly
fetched or posted replies are deduplicated by id before appending to the
existing replies state. Preserve the cursor and hasMore updates, and ensure each
reply appears only once so React keys remain unique.
- Around line 147-149: Update the reply-count state and its consumers in
DoubtRepliesModal so the header and tab badges do not present the paginated
replies.length as the total thread count. Prefer storing and using a total count
returned by the API alongside json.replies; otherwise relabel the displayed
values explicitly as loaded counts.
- Around line 779-784: Update the tie-breaker in the reply sorting comparator
around isPendingA and isPendingB to compare server reply IDs numerically rather
than using String(...).localeCompare(...). Preserve the API’s numeric ID
ordering and provide a fallback comparison for pending IDs that may not be
numeric.
- Around line 158-187: Update loadMoreReplies to associate each request with the
currently active doubt and pagination generation, using an AbortController or
request-generation token. Before applying json.replies, nextCursor, or hasMore,
ignore responses whose doubt.id or generation no longer matches the active modal
state, including requests superseded by a refresh or doubt switch; preserve
loading cleanup for the current request.
- Around line 125-149: Update the replies-loading flow in DoubtRepliesModal so
failed HTTP or JSON responses do not clear previously loaded replies or render
an empty thread. Add an explicit replies error state with retry UI, ensure
loading and error states are represented in the TSX, and only replace replies
with an empty array after a successful response that contains no replies.
- Around line 136-144: Extract the hardcoded error messages and reply-loading
button labels used by DoubtRepliesModal into the project’s existing string
constants or localization source, then replace the inline TSX strings with those
references throughout the component, including the locations around the response
parsing and reply-loading states.
🪄 Autofix (Beta)
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: 03562b4c-a3ba-44e9-8fa6-b4c16ce8f02c
📒 Files selected for processing (1)
src/components/classroom/DoubtRepliesModal.tsx
knoxiboy
left a comment
knoxiboy
left a comment
There was a problem hiding this comment.
Hi @Shreya-nipunge! Thanks for updating the reply pagination.
- Please re-verify that \DoubtRepliesModal.tsx\ fetches strictly page 1 on open.
- Note that \AskAIView.tsx\ still contains a \do { ... } while (!solution && hasMore)\ loop searching for the AI solution reply. While acceptable for solution lookup, please ensure it doesn't cause unnecessary network cascades on initial load.
…tion guards - Remove redundant fetchReplies() call from loadPendingReplies effect - Replace unconditional setReplies([]) with doubt-ID ref for stale guard - Deduplicate Load More responses by reply ID - Use numeric comparison for reply ID sorting
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
github-actions
Bot
removed
the
size:L
|
Hi @knoxiboy , Thanks for the clarification. I re-verified both points against the current implementation.
|
knoxiboy
added
level:advanced
User description
Description
This PR adds cursor-based pagination to the replies API to prevent unbounded database queries when loading doubt threads with a large number of replies.
Previously, the API returned the entire reply history in a single request, which increased response size, memory usage, and page load time for highly active discussions.
Changes
GET /api/replies.cursorandlimitquery parameters (default: 20, maximum: 100).(createdAt, id)keyset predicate to avoid offset drift while maintaining ascending chronological order.limit + 1) to determinehasMorewithout an additionalCOUNTquery.Related Issue
Closes #1062
Type of Change
Screenshots (if UI change)
N/A (Backend/API change)
How Has This Been Tested?
npm run devAdditionally verified:
npx tsc --noEmitpasses.Checklist
npm run dev)anytypes)mainSummary by CodeRabbit
CodeAnt-AI Description
Add paginated reply loading without changing the thread experience
What Changed
hasMoreindicator, avoiding oversized responses for long discussionsImpact
✅ Faster loading for long reply threads✅ Fewer duplicate or missing replies✅ Reliable AI solution display in older discussions💡 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.