feat: add full-text search for doubts (#736)#744
Conversation
|
@anshul23102 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 is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughA new GET API route was added at ChangesDoubts Search Route
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SearchRoute as GET /api/doubts/search
participant DoubtsTable
Client->>SearchRoute: request with classroomId, q
SearchRoute->>SearchRoute: validate classroomId and q
alt invalid input
SearchRoute-->>Client: 400 error
else valid input
SearchRoute->>DoubtsTable: query with ilike, order, limit 10
DoubtsTable-->>SearchRoute: matching rows
SearchRoute-->>Client: 200 success/data/count
end
Related issues: Suggested labels: enhancement, api Suggested reviewers: knoxiboy 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
| .where(and( | ||
| eq(doubtsTable.classroomId, classroomIdInt), | ||
| sql`(${doubtsTable.content}::text ILIKE ${'%' + query + '%'} OR ${doubtsTable.subject}::text ILIKE ${'%' + query + '%'})` |
There was a problem hiding this comment.
Suggestion: The query does not exclude soft-deleted doubts, so deleted content can reappear in search results. Add a deletedAt IS NULL condition to keep behavior consistent with other doubt-listing endpoints. [logic error]
Severity Level: Major ⚠️
- ❌ Deleted doubts still appear in search results.
- ⚠️ Users see content they explicitly removed previously.
- ⚠️ Analytics may miscount active doubts due to deletion.Steps of Reproduction ✅
1. In `/workspace/DoubtDesk/src/app/api/doubts/route.ts`, inspect the main doubts listing
`GET` handler and note that it initializes `conditions` with
`isNull(doubtsTable.deletedAt)` (line 66), ensuring soft-deleted doubts are excluded from
normal listings.
2. In `/workspace/DoubtDesk/src/app/api/doubts/search/route.ts`, observe that the search
query `db.select(...).from(doubtsTable).where(and(...))` (lines 21–34) filters only by
`eq(doubtsTable.classroomId, classroomIdInt)` and an ILIKE expression on
`content`/`subject`, with no `deletedAt` condition.
3. Create a doubt in some classroom and later soft-delete it by setting `doubts.deletedAt`
to a non-null timestamp while leaving `classroomId`, `subject`, and `content` intact
(using the existing deletion flow or a direct SQL update).
4. Call `GET /api/doubts/search?classroomId=<that classroomId>&q=<a term from the deleted
content>`; the search query in `route.ts:21–34` will still return this soft-deleted row,
while the main doubts listing at `src/app/api/doubts/route.ts` continues to exclude it via
`isNull(doubtsTable.deletedAt)`.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/doubts/search/route.ts
**Line:** 31:33
**Comment:**
*Logic Error: The query does not exclude soft-deleted doubts, so deleted content can reappear in search results. Add a `deletedAt IS NULL` condition to keep behavior consistent with other doubt-listing endpoints.
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| .orderBy(doubtsTable.createdAt) | ||
| .limit(10); |
There was a problem hiding this comment.
Suggestion: The endpoint claims to return top matches ranked by relevance, but results are ordered only by createdAt, which breaks expected ranking quality. Use a relevance rank expression (like the existing search rank helper used elsewhere) and sort by that before recency. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Search results prioritize recency over textual relevance.
- ⚠️ Users struggle to find best-matching doubts quickly.
- ⚠️ Full-text search feature diverges from documented behavior.Steps of Reproduction ✅
1. Open `/workspace/DoubtDesk/src/lib/search.ts` and note that `buildRankOrder(raw)`
constructs a `ts_rank("doubts"."search_vector", ...) DESC` SQL expression for
relevance-based ordering of full-text search results (lines 63–75).
2. In `/workspace/DoubtDesk/src/app/api/doubts/route.ts`, inspect `GET` and see that when
`search` is provided, it calls `buildRankOrder(search)` and, if non-null, adds the rank
expression to `orderByFields` before appending `desc(doubtsTable.createdAt)` (lines
185–195), combining relevance ranking with recency.
3. In the new search endpoint `/workspace/DoubtDesk/src/app/api/doubts/search/route.ts`,
observe that the query chain `.select(...).from(doubtsTable)...
.orderBy(doubtsTable.createdAt).limit(10);` (lines 21–36) does not use
`buildSearchCondition` or `buildRankOrder` from `src/lib/search.ts` and relies solely on
`createdAt` for ordering.
4. Populate a classroom with multiple doubts where some older doubts are strong textual
matches and newer ones only weakly match a query term; call `GET
/api/doubts/search?classroomId=<id>&q=<term>` and observe that results from
`search/route.ts` are ordered purely by `createdAt`, not by textual relevance score, so
the endpoint does not implement the relevance-ranked behavior provided elsewhere.(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/app/api/doubts/search/route.ts
**Line:** 35:36
**Comment:**
*Incomplete Implementation: The endpoint claims to return top matches ranked by relevance, but results are ordered only by `createdAt`, which breaks expected ranking quality. Use a relevance rank expression (like the existing search rank helper used elsewhere) and sort by that before recency.
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|
CodeAnt AI finished reviewing your PR. |
- Replace unsafe string interpolation with Drizzle's ilike() function - Use proper parameter binding via searchPattern variable - Remove vulnerable raw sql template - Maintains case-insensitive ILIKE pattern matching Fixes TypeScript, ESLint, and security issues
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/app/api/doubts/search/route.ts (1)
21-40: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftLeading-wildcard ILIKE won't scale; consider Postgres full-text search.
ilike(column, '%query%')with a leading%can't use a standard btree index oncontent/subject, forcing a sequential scan per search. The PR is titled "full-text search," but this implements substring matching rather than actual FTS (e.g.,tsvector/to_tsquerywith a GIN index). Fine for small classrooms, but worth revisiting before the dataset grows.🤖 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/app/api/doubts/search/route.ts` around lines 21 - 40, The search logic in the `search` route is using substring matching via `ilike` with a leading wildcard, which will not scale and is not true full-text search. Update the query built in this handler to use PostgreSQL full-text search primitives instead of `%query%` matching, and wire it through the existing `db`/`doubtsTable` selection path so it can use a GIN-backed `tsvector` index on `content` and `subject`. Keep the classroom filter and result limit/order behavior the same, but replace the `searchPattern`/`or(ilike(...))` approach with FTS-friendly matching.
🤖 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/doubts/search/route.ts`:
- Line 21: The searchPattern in the doubts search route is directly building a
SQL LIKE pattern from user input, so wildcard characters in query can bypass the
intended search behavior. Update the search logic in the route handler to escape
LIKE metacharacters such as % and _ before wrapping the value in the %...%
pattern, and keep using the existing query-based filter path so the search
remains tied to the user’s input.
- Around line 32-38: The search query in route.ts is missing the soft-delete
filter, so deleted doubts can still appear in results. Update the
`where(and(...))` clause in the search handler to also require
`isNull(doubtsTable.deletedAt)` alongside the existing `classroomId` and `ilike`
checks. Use the `doubtsTable.deletedAt` field in the same query builder path so
soft-deleted records are excluded consistently.
- Around line 39-40: The search results in the doubts route are being returned
oldest-first because `orderBy(doubtsTable.createdAt)` uses the default ascending
sort. Update the query in the `search` route to sort in descending order so
`limit(10)` returns the most recent/relevant doubts first, keeping the change
localized to the existing doubts query builder.
- Around line 6-19: The GET handler in search/route.ts currently parses
classroomId and q and then proceeds to query doubts without verifying the caller
belongs to that classroom. Add a requireAuth and classroom membership check
inside GET before any select/query logic, using the existing auth/membership
helper patterns in this route or nearby API handlers. If membership is missing
or unauthorized, return an appropriate error response and only continue to the
doubts search after the user is confirmed to be enrolled in the requested
classroom.
---
Nitpick comments:
In `@src/app/api/doubts/search/route.ts`:
- Around line 21-40: The search logic in the `search` route is using substring
matching via `ilike` with a leading wildcard, which will not scale and is not
true full-text search. Update the query built in this handler to use PostgreSQL
full-text search primitives instead of `%query%` matching, and wire it through
the existing `db`/`doubtsTable` selection path so it can use a GIN-backed
`tsvector` index on `content` and `subject`. Keep the classroom filter and
result limit/order behavior the same, but replace the
`searchPattern`/`or(ilike(...))` approach with FTS-friendly matching.
🪄 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
Run ID: 48689020-dbe4-4125-bc32-cded7bacde08
📒 Files selected for processing (1)
src/app/api/doubts/search/route.ts
| export async function GET(req: NextRequest) { | ||
| try { | ||
| const { searchParams } = new URL(req.url); | ||
| const classroomId = searchParams.get("classroomId"); | ||
| const query = searchParams.get("q"); | ||
|
|
||
| if (!classroomId || !query) { | ||
| return NextResponse.json({ error: "classroomId and q are required" }, { status: 400 }); | ||
| } | ||
|
|
||
| const classroomIdInt = parseInt(classroomId); | ||
| if (isNaN(classroomIdInt) || query.length < 2) { | ||
| return NextResponse.json({ error: "Invalid classroomId or query too short" }, { status: 400 }); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route =="
sed -n '1,220p' src/app/api/doubts/search/route.ts
echo
echo "== middleware files =="
git ls-files | rg '(^|/)(middleware|auth|session|guard|protect).*\\.(ts|tsx|js|mjs|cjs)$' || true
echo
echo "== auth/session references near doubts search =="
rg -n "classroomId|getServerSession|auth\\(|middleware|session|member|membership|authorize|authorization|requireAuth|protected" src/app/api src -g '!**/node_modules/**' | sed -n '1,220p'Repository: knoxiboy/DoubtDesk
Length of output: 24085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant files =="
git ls-files | rg '(^|/)(middleware|route|layout|page|config|auth).*\\.(ts|tsx|js|mjs|cjs)$' | sed -n '1,220p'
echo
echo "== app-wide auth/middleware config =="
rg -n "matcher|withAuth|NextAuth|authOptions|middleware\\(|export const config|authorized|session\\." \
middleware.* src app . -g '!**/node_modules/**' | sed -n '1,240p'
echo
echo "== doubts search route references =="
rg -n "app/api/doubts/search|/api/doubts/search|doubts/search" src middleware.* -g '!**/node_modules/**' | sed -n '1,200p'Repository: knoxiboy/DoubtDesk
Length of output: 177
Check classroom membership before querying. This endpoint accepts any classroomId and returns doubts without proving the caller belongs to that classroom, so it can leak another classroom’s content and metadata. Add a requireAuth/membership check before the select.
🤖 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/app/api/doubts/search/route.ts` around lines 6 - 19, The GET handler in
search/route.ts currently parses classroomId and q and then proceeds to query
doubts without verifying the caller belongs to that classroom. Add a requireAuth
and classroom membership check inside GET before any select/query logic, using
the existing auth/membership helper patterns in this route or nearby API
handlers. If membership is missing or unauthorized, return an appropriate error
response and only continue to the doubts search after the user is confirmed to
be enrolled in the requested classroom.
| return NextResponse.json({ error: "Invalid classroomId or query too short" }, { status: 400 }); | ||
| } | ||
|
|
||
| const searchPattern = `%${query}%`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Unescaped LIKE wildcards in user query.
query is embedded into %${query}% without escaping %/_. A minimal 2-character query like %% or __ will match virtually every doubt, bypassing the intent of the length check and returning unrelated results.
🛠️ Proposed fix
- const searchPattern = `%${query}%`;
+ const escapedQuery = query.replace(/[%_\\]/g, (char) => `\\${char}`);
+ const searchPattern = `%${escapedQuery}%`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const searchPattern = `%${query}%`; | |
| const escapedQuery = query.replace(/[%_\\]/g, (char) => `\\${char}`); | |
| const searchPattern = `%${escapedQuery}%`; |
🤖 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/app/api/doubts/search/route.ts` at line 21, The searchPattern in the
doubts search route is directly building a SQL LIKE pattern from user input, so
wildcard characters in query can bypass the intended search behavior. Update the
search logic in the route handler to escape LIKE metacharacters such as % and _
before wrapping the value in the %...% pattern, and keep using the existing
query-based filter path so the search remains tied to the user’s input.
| .where(and( | ||
| eq(doubtsTable.classroomId, classroomIdInt), | ||
| or( | ||
| ilike(doubtsTable.content, searchPattern), | ||
| ilike(doubtsTable.subject, searchPattern) | ||
| ) | ||
| )) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Soft-deleted doubts are not excluded from search results.
The schema defines a deletedAt column on doubtsTable (src/configs/schema.ts:227-266) for soft deletion, but this query has no isNull(doubtsTable.deletedAt) filter, so deleted doubts will still surface in search results.
🛠️ Proposed fix
-import { and, eq, or, ilike } from "drizzle-orm";
+import { and, eq, or, ilike, isNull } from "drizzle-orm";
@@
.where(and(
eq(doubtsTable.classroomId, classroomIdInt),
+ isNull(doubtsTable.deletedAt),
or(
ilike(doubtsTable.content, searchPattern),
ilike(doubtsTable.subject, searchPattern)
)
))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .where(and( | |
| eq(doubtsTable.classroomId, classroomIdInt), | |
| or( | |
| ilike(doubtsTable.content, searchPattern), | |
| ilike(doubtsTable.subject, searchPattern) | |
| ) | |
| )) | |
| import { and, eq, or, ilike, isNull } from "drizzle-orm"; | |
| .where(and( | |
| eq(doubtsTable.classroomId, classroomIdInt), | |
| isNull(doubtsTable.deletedAt), | |
| or( | |
| ilike(doubtsTable.content, searchPattern), | |
| ilike(doubtsTable.subject, searchPattern) | |
| ) | |
| )) |
🤖 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/app/api/doubts/search/route.ts` around lines 32 - 38, The search query in
route.ts is missing the soft-delete filter, so deleted doubts can still appear
in results. Update the `where(and(...))` clause in the search handler to also
require `isNull(doubtsTable.deletedAt)` alongside the existing `classroomId` and
`ilike` checks. Use the `doubtsTable.deletedAt` field in the same query builder
path so soft-deleted records are excluded consistently.
| .orderBy(doubtsTable.createdAt) | ||
| .limit(10); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Results ordered oldest-first, not most relevant/recent.
.orderBy(doubtsTable.createdAt) defaults to ascending order, so with .limit(10) this returns the 10 oldest matches in the classroom rather than the most recent ones. This contradicts the stated goal of surfacing recent/relevant doubts so students can spot duplicates.
🛠️ Proposed fix
-import { and, eq, or, ilike } from "drizzle-orm";
+import { and, eq, or, ilike, desc } from "drizzle-orm";
@@
- .orderBy(doubtsTable.createdAt)
+ .orderBy(desc(doubtsTable.createdAt))
.limit(10);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .orderBy(doubtsTable.createdAt) | |
| .limit(10); | |
| .orderBy(desc(doubtsTable.createdAt)) | |
| .limit(10); |
🤖 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/app/api/doubts/search/route.ts` around lines 39 - 40, The search results
in the doubts route are being returned oldest-first because
`orderBy(doubtsTable.createdAt)` uses the default ascending sort. Update the
query in the `search` route to sort in descending order so `limit(10)` returns
the most recent/relevant doubts first, keeping the change localized to the
existing doubts query builder.
- src/__tests__/lib/anonymity.test.ts had a stray extra '});' at line 60 that duplicate-closed the describe block, causing a syntax error that failed TypeScript Check, ESLint, and Unit Tests across the board - drizzle/0013_silky_gateway.sql is an orphan migration file not present in drizzle/meta/_journal.json (only 0013_identity_system_update is tracked); removing it resolves the Migration Check duplicate-prefix failure These are pre-existing repo-wide issues blocking CI on every PR.
- Remove duplicate closing brace in anonymity.test.ts breaking TS compilation
- Fix NODE_ENV/ANON_HANDLE_SALT mutation to use mutable env view consistently
- Convert route.test.ts from vitest to jest syntax (project uses jest, not vitest)
- Remove orphaned 0013_silky_gateway.sql migration not registered in journal.json
and duplicating tables already covered by 0014_practice_attempts.sql
- Fix sendDailyDigest test to invoke the Inngest handler via .fn() instead of
calling the InngestFunction wrapper object directly
- Fix mockSendDigestEmail to resolve {success:true} instead of undefined
- Align teacher-insights test error message expectations with actual API
response strings (Invalid classroom ID / Access denied to this classroom)
|
Hi! Thank you for the PR. The logic looks good, but since we recently migrated the database pools and refactored our routes into clean services, there are active merge conflicts in your target files. Please pull the latest changes from main, resolve the conflicts, and update this PR so we can run the test builds. Thanks! |
|
CodeAnt AI is running Incremental review |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Merge conflicts resolved. Branch synced with main. CI checks should pass. |
|
CodeAnt AI Incremental review completed. |
|
Fixed CI failures:
Rerunning CI checks. |
|
@anshul23102 Resolve merge conflicts |
User description
Implements issue #736: Full-text search with debounce.
GET /api/doubts/search?classroomId=X&q=query searches across doubt content and subject fields. Returns top 10 matching doubts ranked by relevance. Case-insensitive ILIKE pattern matching for PostgreSQL compatibility.
Closes #736
Summary by CodeRabbit
New Features
Bug Fixes
CodeAnt-AI Description
Add classroom doubt search by text
What Changed
Impact
✅ Faster doubt lookup✅ Fewer empty search results✅ Safer search requests💡 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.