feat: add bookmarks, exports, and public room workflows#642
Open
saurabhhhcodes wants to merge 2 commits into
Open
feat: add bookmarks, exports, and public room workflows#642saurabhhhcodes wants to merge 2 commits into
saurabhhhcodes wants to merge 2 commits into
Conversation
|
@saurabhhhcodes 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 · |
github-actions
Bot
added
gssoc'26
Contributor
WalkthroughThis PR introduces a comprehensive tagging system for doubts with classroom-scoped access control, adds vision-based image quality detection for AI responses, and updates the UI to support tag suggestions, filtering, and display across doubt creation, editing, and browsing workflows. ChangesTag System Core
Vision Image Quality Detection
Frontend Tag Support and UI
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⭐ Star Required
Hi @saurabhhhcodes! Thank you for your contribution to DoubtDesk.
Before we can complete the review and merge this pull request, please star the DoubtDesk repository.
Once you have starred the repository, please drop a comment here saying "done" and we will proceed with reviewing your PR. Thank you!
|
@coderabbitai review |
codeant-ai
Bot
added
the
size:XL
github-actions
Bot
removed
the
size:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| const { userName, subject, content, imageUrl, classroomId, type = 'community' } = await req.json(); | ||
| const { userName, subject, content, imageUrl, classroomId, type = 'community', tags = [] } = await req.json(); | ||
| const parsedClassroomId = classroomId ? parseInt(classroomId.toString()) : null; |
There was a problem hiding this comment.
Suggestion: parseInt output is used directly without validating Number.isNaN, so malformed input like "abc" becomes NaN and can cause a database type failure at insert time (500). Validate the parsed value and return a 400 for invalid IDs before using it. [type error]
Severity Level: Major ⚠️
- ❌ Malformed classroomId causes 500 on doubt creation endpoint.
- ⚠️ API consumers can't distinguish bad input from server failures.Steps of Reproduction ✅
1. Run the API with this PR so `POST /api/doubts` is handled by
`app/api/doubts/route.ts:171`.
2. Authenticate as any valid user (so `currentUser()` at lines 171-177 returns a user with
a non-null email).
3. Send `POST /api/doubts` with JSON where `classroomId` is a non-numeric string, e.g. `{
"userName": "Student_1", "subject": "Math", "content": "Q", "classroomId": "abc" }`.
4. In the handler, `classroomId` is truthy so `parsedClassroomId =
parseInt(classroomId.toString())` at line 189 becomes `NaN`. This `NaN` is passed as
`classroomId` into the `doubtsTable` insert at lines 207-215. Because
`doubtsTable.classroomId` is an `integer()` column (`configs/schema.ts:117`) and
`tagsTable` lookups also use this value (lines 226-229), the database driver will reject
`NaN` with a type error, causing the catch block at lines 247-250 to return a 500
`Internal Server Error` instead of a 400 for invalid input.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/doubts/route.ts
**Line:** 189:189
**Comment:**
*Type Error: `parseInt` output is used directly without validating `Number.isNaN`, so malformed input like `"abc"` becomes `NaN` and can cause a database type failure at insert time (500). Validate the parsed value and return a 400 for invalid IDs before using it.
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
Comment on lines
+218
to
+222
| const normalizedTags: string[] = Array.from(new Set( | ||
| (Array.isArray(tags) ? tags : []) | ||
| .map((tag: string) => tag.trim().replace(/\s+/g, " ").toLowerCase()) | ||
| .filter(Boolean) | ||
| )).slice(0, 8); |
There was a problem hiding this comment.
Suggestion: The tags normalization assumes every element is a string and calls trim() directly; a crafted payload with non-string entries (number/object/null) will throw at runtime and return 500. Guard with a type check before trimming or coerce safely. [type error]
Severity Level: Major ⚠️
- ❌ Non-string tags crash POST /api/doubts handler.
- ⚠️ Malicious payloads can trigger unnecessary 500 server errors.Steps of Reproduction ✅
1. Start the service so `POST /api/doubts` is implemented by
`app/api/doubts/route.ts:171-252`.
2. Authenticate as a valid user (so email is set at lines 171-178) and send `POST
/api/doubts` with a JSON body where `tags` contains non-string entries, e.g. `"tags":
["algebra", 123, null]`, along with valid `userName`, `subject`, and `content`.
3. The handler inserts the new doubt at lines 207-215, then builds `normalizedTags` at
lines 218-222 by doing `(Array.isArray(tags) ? tags : []).map((tag: string) =>
tag.trim()...)`. At runtime, the type annotation `(tag: string)` is erased, so when it
encounters `123` or `null`, it attempts to call `.trim()` on a non-string value.
4. This causes a `TypeError: tag.trim is not a function` inside the POST handler loop at
line 220, which bubbles to the outer `try`/`catch` (lines 171-252). The catch at lines
247-250 logs the error and returns a 500 `Internal Server Error`, breaking the endpoint
for such payloads instead of validating and returning a 400.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/doubts/route.ts
**Line:** 218:222
**Comment:**
*Type Error: The tags normalization assumes every element is a string and calls `trim()` directly; a crafted payload with non-string entries (number/object/null) will throw at runtime and return 500. Guard with a type check before trimming or coerce safely.
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
Comment on lines
+226
to
+238
| const [existingTag] = await db.select().from(tagsTable).where(and( | ||
| eq(tagsTable.normalizedName, normalizedName), | ||
| parsedClassroomId ? eq(tagsTable.classroomId, parsedClassroomId) : isNull(tagsTable.classroomId) | ||
| )).limit(1); | ||
|
|
||
| const [tagRecord] = existingTag | ||
| ? [existingTag] | ||
| : await db.insert(tagsTable).values({ | ||
| name: normalizedName.replace(/\b\w/g, (char) => char.toUpperCase()), | ||
| normalizedName, | ||
| classroomId: parsedClassroomId, | ||
| createdByEmail: email, | ||
| }).returning(); |
There was a problem hiding this comment.
Suggestion: The tag upsert flow has a race condition: two concurrent requests can both miss existingTag and both attempt insert, hitting the unique index and failing one request with 500. Replace select-then-insert with a single atomic upsert/on-conflict strategy and fetch the resulting row. [race condition]
Severity Level: Major ⚠️
- ❌ Concurrent identical tags sometimes fail with 500 errors.
- ⚠️ Tag creation unreliable under parallel classroom usage spikes.Steps of Reproduction ✅
1. Note that `tagsTable` enforces a unique scope on `(normalizedName, classroomId)` via
`uniqueIndex("tag_scope_name_idx")` in `configs/schema.ts:140-150`.
2. Run the API server so `POST /api/doubts` uses the tag upsert loop at
`app/api/doubts/route.ts:224-245`.
3. From two separate clients (or threads) authenticated as valid users, concurrently send
`POST /api/doubts` to the same classroom with an identical new tag, e.g. `"tags":
["algebra"]`, where no existing tag with `normalizedName = "algebra"` exists yet for that
`classroomId`.
4. In each request, the loop over `normalizedTags` (lines 225-245) runs: the `select` at
lines 226-229 returns no rows in both requests, so `existingTag` is undefined for both,
and each proceeds to execute `db.insert(tagsTable).values(...).returning()` at lines
233-238. The first insert succeeds, creating the tag; the second hits the unique index
`tag_scope_name_idx` and raises a unique violation, causing that POST to enter the catch
block at lines 247-250 and return a 500 error even though the payload was valid.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/doubts/route.ts
**Line:** 226:238
**Comment:**
*Race Condition: The tag upsert flow has a race condition: two concurrent requests can both miss `existingTag` and both attempt insert, hitting the unique index and failing one request with 500. Replace select-then-insert with a single atomic upsert/on-conflict strategy and fetch the resulting row.
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
Comment on lines
+137
to
+146
| <input | ||
| type="text" | ||
| placeholder="Filter by tag..." | ||
| value={tagFilter} | ||
| onChange={(e) => setTagFilter(e.target.value)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") fetchDoubts(); | ||
| }} | ||
| className="bg-slate-900 border border-white/10 rounded-xl px-4 py-2 text-[10px] font-bold text-white placeholder:text-slate-600 focus:outline-none focus:border-blue-500 transition-all w-40" | ||
| /> |
There was a problem hiding this comment.
🎨 Design Review — MEDIUM
Do you think the new tag-filter input would be easier for screen-reader users if it had an explicit label (or aria-label), instead of relying only on placeholder text?
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a **Design Review** comment — a question about the UX/design of frontend code. It is intentionally framed as a question, not a prescription. The author may agree or disagree.
**Path:** app/public-rooms/page.tsx
**Line:** 137:146
**Comment:**
*MEDIUM: Do you think the new tag-filter input would be easier for screen-reader users if it had an explicit label (or aria-label), instead of relying only on placeholder text?
- If you agree with the proposal, apply a small, localized change (swap a color token, bump a font size, adjust spacing, add an aria-label, etc.).
- If you disagree, or the answer depends on a design decision a human should make, explain your reasoning and ask the user how to proceed.
Do NOT refactor surrounding components or apply other design changes that weren't asked about.
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS "tag_classroomId_idx" ON "tags" ("classroomId"); | ||
| CREATE UNIQUE INDEX IF NOT EXISTS "tag_scope_name_idx" ON "tags" ("normalizedName", "classroomId"); |
There was a problem hiding this comment.
Suggestion: The unique index on (normalizedName, classroomId) does not prevent duplicates when classroomId is NULL (PostgreSQL treats NULL values as distinct in unique indexes). This allows multiple identical public tags and breaks the intended "one tag per scope" rule. Use a separate partial unique index for public tags (classroomId IS NULL) or make scope explicit with a non-null sentinel value. [logic error]
Severity Level: Major ⚠️
- ⚠️ Duplicate public tags appear in `/api/tags` responses.
- ⚠️ Tag autocomplete may show the same tag multiple times.
- ⚠️ Future tag uniqueness assumptions in code become unreliable.Steps of Reproduction ✅
1. Note from `configs/schema.ts:11-21` that `tagsTable` is defined with `classroomId`
nullable and a unique index `tag_scope_name_idx` on `(normalizedName, classroomId)`;
public tags are documented as having `classroomId = null` (comments at
`configs/schema.ts:7-10`).
2. See the migration `drizzle/0001_add_doubt_tags.sql:1-11` where the database-level index
`CREATE UNIQUE INDEX IF NOT EXISTS "tag_scope_name_idx" ON "tags" ("normalizedName",
"classroomId");` is created, matching the Drizzle schema.
3. Create two concurrent HTTP requests to `POST /api/tags` implemented in
`app/api/tags/route.ts:58-92`, both with the same JSON body `{ "name": "algebra",
"classroomId": null }` for a logged-in user. Each handler call normalizes the name (line
65), computes `classroomId = null` (line 66), checks for an existing tag with
`eq(tagsTable.normalizedName, normalizedName)` and `isNull(tagsTable.classroomId)` (lines
76-80), and if nothing is returned, inserts a new row into `tagsTable` (lines 85-90).
4. Because PostgreSQL unique indexes treat NULLs as distinct and the index is on
`(normalizedName, classroomId)`, the check in step 3 can race: both concurrent requests
see no existing row and both insert a row with `(normalizedName='algebra',
classroomId=NULL)`. The unique index at `drizzle/0001_add_doubt_tags.sql:11` does not
prevent this, so the database ends up with duplicate public tags. A subsequent `GET
/api/tags` call (implemented in `app/api/tags/route.ts:22-51`) with no `classroomId` will
query only `isNull(tagsTable.classroomId)` (lines 37-41) and return multiple identical
rows for the same public tag.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** drizzle/0001_add_doubt_tags.sql
**Line:** 11:11
**Comment:**
*Logic Error: The unique index on (`normalizedName`, `classroomId`) does not prevent duplicates when `classroomId` is `NULL` (PostgreSQL treats `NULL` values as distinct in unique indexes). This allows multiple identical public tags and breaks the intended "one tag per scope" rule. Use a separate partial unique index for public tags (`classroomId IS NULL`) or make scope explicit with a non-null sentinel value.
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
Comment on lines
+91
to
+92
| if (detectedSubject && !subjectWasEdited) { | ||
| setSubject(detectedSubject); |
There was a problem hiding this comment.
Suggestion: Auto-subject detection currently runs during edit mode and can overwrite an existing doubt's subject before the user changes it. Since subjectWasEdited starts as false, opening an existing doubt with long content can silently replace its original subject and save incorrect data. Skip auto-assign when doubtToEdit is present (or initialize edit mode as already manually set). [logic error]
Severity Level: Critical 🚨
- ❌ Editing doubts can silently change their subject.
- ⚠️ Subject-based filters return misclassified doubts after edits.
- ⚠️ Classroom views show incorrect subjects for existing doubts.Steps of Reproduction ✅
1. Open any page that renders `DoubtCard` (components/DoubtCard.tsx:16-80), for example a
classroom view using GET `/api/doubts` (app/api/doubts/route.ts:9-165), so that an
existing doubt with non-empty `subject` and long `content` is displayed.
2. As the owner of that doubt, click the Edit button rendered at
components/DoubtCard.tsx:29-39 (`onClick={() => setIsEditModalOpen(true)}`), which opens
the `AskDoubt` modal in edit mode with `doubtToEdit={doubt}` at
components/DoubtCard.tsx:60-70.
3. When `AskDoubt` mounts in edit mode, the effect at components/AskDoubt.tsx:68-75 sets
`content` and `subject` from `doubtToEdit`, but `subjectWasEdited` remains `false`
(initialized at line 65 and only reset to `false` for new doubts in the `else` branch at
line 78). The auto-subject effect at components/AskDoubt.tsx:82-94 then runs: for content
containing a keyword (e.g. "force" or "integral") it computes `detectedSubject =
suggestSubject(content)` at line 88 and, because `!subjectWasEdited` is still true,
executes `setSubject(detectedSubject)` at lines 91-92, silently overwriting the original
subject.
4. Without manually touching the subject field, click "Save Changes" in the modal, which
triggers `handleSubmit` at components/AskDoubt.tsx:151-162 and sends a PATCH request
`/api/doubts/action/{id}` with `{ action: "edit", content, subject, imageUrl, tags }`. The
backend handler at app/api/doubts/action/[id]/route.ts:115-127 updates
`doubtsTable.subject` to this auto-detected value, permanently changing the doubt's
subject away from what was originally saved.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** components/AskDoubt.tsx
**Line:** 91:92
**Comment:**
*Logic Error: Auto-subject detection currently runs during edit mode and can overwrite an existing doubt's subject before the user changes it. Since `subjectWasEdited` starts as false, opening an existing doubt with long content can silently replace its original subject and save incorrect data. Skip auto-assign when `doubtToEdit` is present (or initialize edit mode as already manually set).
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| createdAt: timestamp().defaultNow().notNull(), | ||
| }, (table) => ({ | ||
| classroomIdIndex: index("tag_classroomId_idx").on(table.classroomId), | ||
| normalizedNameIndex: uniqueIndex("tag_scope_name_idx").on(table.normalizedName, table.classroomId), |
There was a problem hiding this comment.
Suggestion: This uniqueness rule does not actually enforce uniqueness for public tags because classroomId is nullable and PostgreSQL treats NULL values as distinct in unique indexes. That allows duplicate public tags with the same normalized name, causing inconsistent tag matching/filtering. Use a null-safe uniqueness strategy (for example, separate partial unique indexes for classroomId IS NULL and classroomId IS NOT NULL, or a functional index with coalesce). [logic error]
Severity Level: Major ⚠️
- ⚠️ Concurrent tag creation can duplicate public tag names.
- ⚠️ Tag listings may show the same public tag twice.
- ⚠️ Doubt-tag joins can bind to inconsistent tag ids.Steps of Reproduction ✅
1. Note the schema for `tagsTable` in configs/schema.ts:140-150, which defines a unique
index `tag_scope_name_idx` on `(normalizedName, classroomId)` via
`uniqueIndex("tag_scope_name_idx").on(table.normalizedName, table.classroomId)` while
allowing `classroomId` to be nullable (line 144 and drizzle migration
drizzle/0001_add_doubt_tags.sql:1-11).
2. Observe the tag creation logic in app/api/tags/route.ts:58-92: the POST handler
normalizes the name, checks for an existing tag with `eq(tagsTable.normalizedName,
normalizedName)` and either `eq(tagsTable.classroomId, classroomId)` or
`isNull(tagsTable.classroomId)` at lines 76-80, and if none is found inserts a new row
with `classroomId` potentially `null`.
3. Trigger two concurrent POST `/api/tags` requests with identical `name` and no
`classroomId` (public tag) so both run through the `existing` lookup at
app/api/tags/route.ts:76-81 before either insert commits; each sees no existing row and
proceeds to insert a tag with the same `normalizedName` and `classroomId = null`.
4. Because PostgreSQL unique indexes treat `NULL` values as distinct, the unique index
`tag_scope_name_idx` on `(normalizedName, classroomId)` (configs/schema.ts:147-150 /
drizzle/0001_add_doubt_tags.sql:11) does not reject these duplicate pairs where
`classroomId` is `NULL`, allowing multiple public tags with the same `normalizedName`.
Subsequent queries that join tags by normalizedName, such as the doubt-tag linking in
app/api/doubts/route.ts:224-239 or tag listing in app/api/tags/route.ts:37-49, can now
encounter multiple rows representing what should be a single logical tag.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** configs/schema.ts
**Line:** 149:149
**Comment:**
*Logic Error: This uniqueness rule does not actually enforce uniqueness for public tags because `classroomId` is nullable and PostgreSQL treats `NULL` values as distinct in unique indexes. That allows duplicate public tags with the same normalized name, causing inconsistent tag matching/filtering. Use a null-safe uniqueness strategy (for example, separate partial unique indexes for `classroomId IS NULL` and `classroomId IS NOT NULL`, or a functional index with `coalesce`).
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
| function isUnclearVisionReply(reply: string) { | ||
| const normalizedReply = reply.replace(/^SUBJECT:\s*.+\n?/im, '').trim(); | ||
|
|
||
| return normalizedReply.length < 50 || UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply)); |
There was a problem hiding this comment.
Suggestion: The unclear-image detector treats any vision reply under 50 characters as a failed OCR result, which will incorrectly reject valid short answers (for example, simple numeric/image questions) with HTTP 422. Remove the hard length cutoff and rely on explicit unclear-image signals instead. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Vision Ask-AI responses under 50 chars rejected.
- ⚠️ Simple image questions can never return concise answers.Steps of Reproduction ✅
1. Navigate to a classroom page rendered by `ClassroomPage` in
`app/rooms/[id]/page.tsx:51-60` and open the "Ask AI" tab, which mounts `AskAIView` with
`classroomId={Number(id)}` (lines 257-267).
2. In `AskAIView` (`components/AskAIView.tsx:171-186`), switch to the "Upload Image" mode
via the button at lines 180-185 and upload an image; `handleImageUpload` at lines 132-138
sets `imageBase64` while `prompt` may be empty.
3. Click the primary "Solve Scoped" button, which calls `handleAskAI('standard')` at
`components/AskAIView.tsx:239-253`. This issues a POST to `/api/ask-ai` with `{ prompt,
type, imageBase64, classroomId }` as seen at lines 148-152.
4. The `/api/ask-ai` POST handler in `app/api/ask-ai/route.ts:89-189` detects
`isVisionRequest = !!imageBase64 && !isFollowUp` (line 186), builds messages, calls
`callGroqWithFallback` (lines 218-219), and obtains `reply`. If Groq returns any
legitimate short answer under 50 characters for this vision request,
`isUnclearVisionReply(reply)` at lines 43-47 returns `true` because of
`normalizedReply.length < 50`, causing the handler at lines 221-225 to return HTTP 422
with `{ error: IMAGE_QUALITY_ERROR, code: "IMAGE_QUALITY_LOW" }` instead of the valid
answer. `AskAIView.handleAskAI` then treats this as an error (lines 153-161), sets
`errorMsg`, and never shows the (discarded) correct reply.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/ask-ai/route.ts
**Line:** 46:46
**Comment:**
*Incorrect Condition Logic: The unclear-image detector treats any vision reply under 50 characters as a failed OCR result, which will incorrectly reject valid short answers (for example, simple numeric/image questions) with HTTP 422. Remove the hard length cutoff and rely on explicit unclear-image signals instead.
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
Comment on lines
+136
to
+158
| await db.delete(doubtTagsTable).where(eq(doubtTagsTable.doubtId, doubtId)); | ||
|
|
||
| const savedTags: any[] = []; | ||
| for (const normalizedName of normalizedTags) { | ||
| const [existingTag] = await db.select().from(tagsTable).where(and( | ||
| eq(tagsTable.normalizedName, normalizedName), | ||
| doubt.classroomId ? eq(tagsTable.classroomId, doubt.classroomId) : isNull(tagsTable.classroomId) | ||
| )).limit(1); | ||
|
|
||
| const [tagRecord] = existingTag | ||
| ? [existingTag] | ||
| : await db.insert(tagsTable).values({ | ||
| name: normalizedName.replace(/\b\w/g, (char) => char.toUpperCase()), | ||
| normalizedName, | ||
| classroomId: doubt.classroomId, | ||
| createdByEmail: email || null, | ||
| }).returning(); | ||
|
|
||
| savedTags.push(tagRecord); | ||
| await db.insert(doubtTagsTable).values({ | ||
| doubtId, | ||
| tagId: tagRecord.id, | ||
| }).onConflictDoNothing(); |
There was a problem hiding this comment.
Suggestion: Tag replacement is not atomic: existing tag links are deleted before new links are fully recreated, so any failure mid-loop leaves the doubt with partial or empty tags. Wrap delete+upsert+link operations in a single database transaction to avoid data loss on errors. [logic error]
Severity Level: Major ⚠️
- ⚠️ Edited doubts can permanently lose all associated tags.
- ⚠️ Tag-based classroom filters return inconsistent, partial doubt sets.Steps of Reproduction ✅
1. A user edits an existing doubt via the `AskDoubt` modal in edit mode.
`components/AskDoubt.tsx:56-75` initializes from `doubtToEdit`, and `handleSubmit` at
lines 151-162 sends a PATCH request to `/api/doubts/action/${doubtToEdit.id}` with body `{
action: "edit", content, subject, imageUrl, tags }`.
2. The PATCH handler in `app/api/doubts/action/[id]/route.ts:7-15` parses the `id`, loads
the doubt (lines 20-21), checks permissions (lines 23-31), and enters the `action ===
"edit"` branch at lines 115-162.
3. Inside the edit branch, the code updates the core doubt fields via
`db.update(doubtsTable)...returning()` (lines 121-128), normalizes the incoming tag list
into `normalizedTags` (lines 130-134), then deletes all existing tag links for this doubt
with `await db.delete(doubtTagsTable)...` at line 136.
4. The function then iterates `normalizedTags` (lines 139-158), potentially creating new
rows in `tagsTable` and re-creating links in `doubtTagsTable`. If any DB operation inside
this loop throws (for example, insert to `tagsTable` or `doubtTagsTable` fails mid-loop),
control jumps to the catch block at lines 165-167, returning a 500. The prior delete at
line 136 is not wrapped in a transaction, so subsequent GETs through
`app/api/doubts/route.ts:9-164` will see the doubt with partially recreated or completely
missing tags because `doubtTagsTable` links were removed and only some (or none) were
reinserted.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/doubts/action/[id]/route.ts
**Line:** 136:158
**Comment:**
*Logic Error: Tag replacement is not atomic: existing tag links are deleted before new links are fully recreated, so any failure mid-loop leaves the doubt with partial or empty tags. Wrap delete+upsert+link operations in a single database transaction to avoid data loss on errors.
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
Comment on lines
+65
to
+73
| const normalizedName = normalizeTagName(name || ""); | ||
| const classroomId = rawClassroomId ? parseInt(rawClassroomId.toString()) : null; | ||
|
|
||
| if (!normalizedName) { | ||
| return NextResponse.json({ error: "Tag name is required" }, { status: 400 }); | ||
| } | ||
|
|
||
| if (classroomId && !(await canAccessClassroom(classroomId, email))) { | ||
| return NextResponse.json({ error: "Access denied to this classroom" }, { status: 403 }); |
There was a problem hiding this comment.
Suggestion: In tag creation, malformed classroomId values are converted to NaN and then treated as null, which can create a global tag instead of rejecting bad input. Validate parsed IDs before scope checks and insert operations. [logic error]
Severity Level: Major ⚠️
- ⚠️ Malformed classroomId skips membership checks for tag creation.
- ⚠️ Bad IDs can cause 500s instead of clear 400 validation errors.Steps of Reproduction ✅
1. Send an authenticated POST to `/api/tags` handled by the `POST` function in
`app/api/tags/route.ts:58-97` with body `{ "name": "Vectors", "classroomId": "abc" }`.
2. The handler reads `rawClassroomId` from the request body at line 64 and computes `const
classroomId = rawClassroomId ? parseInt(rawClassroomId.toString()) : null;` at line 66,
which parses `"abc"` into `NaN`.
3. Since `normalizedName` is non-empty (`"vectors"`), the name validation at lines 68-70
passes, and the membership check `if (classroomId && !(await
canAccessClassroom(classroomId, email)))` at lines 72-74 is skipped because `classroomId`
is `NaN` (falsy), so no classroom-access validation occurs for this malformed ID.
4. The uniqueness query at lines 76-81 uses `classroomId ? eq(tagsTable.classroomId,
classroomId) : isNull(tagsTable.classroomId)`; with `classroomId` as `NaN`, the falsy
branch treating it like "no classroom" is taken, and the subsequent insert at lines 85-90
uses `classroomId` (i.e., `NaN`) as the value. Depending on the database driver, this can
either create a tag effectively in the global scope without ever validating any classroom
membership, or throw a server-side error that is surfaced as HTTP 500 (lines 93-96)
instead of a client-visible 400 for invalid classroomId input.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/api/tags/route.ts
**Line:** 65:73
**Comment:**
*Logic Error: In tag creation, malformed `classroomId` values are converted to `NaN` and then treated as `null`, which can create a global tag instead of rejecting bad input. Validate parsed IDs before scope checks and insert operations.
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
Comment on lines
+123
to
+130
| const params = new URLSearchParams({ | ||
| classroomId: id?.toString() || "", | ||
| userName: userName || "", | ||
| type, | ||
| }); | ||
| if (tagFilter.trim()) params.append("tag", tagFilter.trim()); | ||
|
|
||
| const res = await fetch(`/api/doubts?${params.toString()}`); |
There was a problem hiding this comment.
Suggestion: The new tag filter is applied to every tab fetch, including Ask-AI, but the Ask-AI view has no filter controls; this causes hidden stale filter state to unexpectedly suppress AI results after users filter community/teacher tabs. Scope tag filtering only to tabs where the filter UI is present or reset it when changing tabs. [logic error]
Severity Level: Major ⚠️
- ⚠️ Ask-AI history silently filtered by hidden tag state.
- ⚠️ New AI doubts may not appear despite successful creation.Steps of Reproduction ✅
1. Open a classroom at `/rooms/[id]` rendered by `ClassroomPage` in
`app/rooms/[id]/page.tsx:51-60`. The component initializes shared state including
`activeTab`, `doubts`, and `tagFilter` at lines 56-67.
2. Switch to the "Community" tab using the navigation buttons at lines 230-247. In the
community header (`activeTab === "community"` block at lines 52-104 of the later section),
type a tag into the "Filter tag" input bound to `tagFilter` at lines 80-86; this updates
the shared `tagFilter` state while the community tab is active.
3. The `useEffect` at lines 149-160 depends on `[activeTab, tagFilter]`. With `activeTab
=== "community"` and non-empty `tagFilter`, it calls `fetchScopedDoubts(type)` (line 159).
`fetchScopedDoubts` at lines 119-147 builds `params` with classroomId, userName, and
`type`, and at line 128 appends `tag` when `tagFilter.trim()` is non-empty, so the
community list is intentionally filtered.
4. Now click the "Ask AI" tab in the header (lines 232-247) to set `activeTab` to
`"ask-ai"`. The same `tagFilter` value persists, but there is no tag filter UI rendered
within the Ask-AI tab (lines 2-49 of the Ask-AI section). The `useEffect` runs again with
`activeTab === "ask-ai"` and non-empty `tagFilter`, causing `fetchScopedDoubts('ai')` to
be called (line 158). That function uses the same `tagFilter` to append `tag` at line 128,
so `/api/doubts?type=ai&tag=<previous>` is fetched. If no AI doubts carry that tag,
`doubts` becomes empty and the "Neural Resolve History" list in the Ask-AI tab (lines
15-47) incorrectly shows no AI history, and new AI queries triggered via `AskAIView`'s
`onSuccess={() => fetchScopedDoubts('ai')}` (lines 7-11 of the Ask-AI block) also fail to
appear until the user returns to a tab that exposes the tag filter and manually clears it.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** app/rooms/[id]/page.tsx
**Line:** 123:130
**Comment:**
*Logic Error: The new tag filter is applied to every tab fetch, including Ask-AI, but the Ask-AI view has no filter controls; this causes hidden stale filter state to unexpectedly suppress AI results after users filter community/teacher tabs. Scope tag filtering only to tabs where the filter UI is present or reset it when changing tabs.
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
app/api/ask-ai/route.ts (2)
109-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the request body with Zod before using it.
This handler still trusts
req.json()shape. A malformedhistoryvalue can later breakmessages.push(...history), and invalidtype,classroomId, orimageBase64values flow into the model/database path unchecked. Return a400on schema failure instead of letting bad payloads reach the expensive path. As per coding guidelines,app/api/**: Input validation (use Zod schemas).🧩 Minimal schema pattern
+import { z } from 'zod'; + +const AskAiRequestSchema = z.object({ + prompt: z.string().optional().default(''), + type: z.enum(['standard', 'simple', 'exam', 'eli10']).optional().default('standard'), + imageBase64: z.string().optional().nullable(), + classroomId: z.coerce.number().int().positive().optional().nullable(), + history: z.array(z.object({ + role: z.enum(['user', 'assistant']), + content: z.string().min(1), + })).optional().default([]), +}); ... - const body = await req.json(); - const { prompt, type = 'standard', imageBase64, classroomId, history = [] } = body; + const parsed = AskAiRequestSchema.safeParse(await req.json()); + if (!parsed.success) { + return NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); + } + const { prompt, type, imageBase64, classroomId, history } = parsed.data;🤖 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 `@app/api/ask-ai/route.ts` around lines 109 - 123, Add Zod validation for the incoming body before using req.json() values: define a Zod schema that validates prompt (optional string), type (enum 'standard' | ... default 'standard'), imageBase64 (optional string), classroomId (optional string), and history (array of objects with role: 'user'|'assistant' and content: string). Replace the direct destructure of req.json() with zodParse/tryParse on the parsed body and return NextResponse.json({ error: validation.errors }, { status: 400 }) on failure; then use the validated values for subsequent calls (moderateContent, handleModerationViolation, messages.push(...history), DB/model calls) to prevent malformed payloads from reaching those paths.Source: Coding guidelines
89-118:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd rate limiting before this route hits moderation and Groq.
An authenticated caller can currently drive moderation plus up to three upstream model attempts per request. On a route this expensive, one abusive or buggy client can burn quota and degrade the feature for everyone else. Throttle by user/IP before the external calls. As per coding guidelines,
app/api/**: Rate limiting on sensitive endpoints.🤖 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 `@app/api/ask-ai/route.ts` around lines 89 - 118, Add a rate-limiting check at the very start of the POST handler (before calling moderateContent, Groq queries, or any upstream model attempts) to throttle by user (email) and fallback to IP; if the caller exceeds their quota return a 429 JSON response. Implement this by invoking your existing rate limiter utility (or add a token-bucket/redis-limiter) using the user email (from email) or request IP as the key, and short-circuit the request inside POST when the check fails; ensure the check happens before calls to moderateContent and any functions that perform Groq/upstream model calls so abusive clients cannot drive moderation or model quota.Source: Coding guidelines
app/ask-ai/page.tsx (1)
379-428:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAnnounce the new error state to assistive tech in both ask-AI UIs.
These banners are inserted after the request fails, but neither one is a live region. Screen-reader users can stay focused in the form and never hear that
IMAGE_QUALITY_LOWor another failure was rendered. Addrole="alert"(oraria-live="assertive") to the banner container in bothapp/ask-ai/page.tsxandcomponents/AskAIView.tsx. As per coding guidelines,**/*.tsx: Review for: Accessibility (aria labels, keyboard navigation).🤖 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 `@app/ask-ai/page.tsx` around lines 379 - 428, The error banner currently rendered when errorMsg is set (the div with className "flex items-start gap-3 p-4 bg-red-500/10 ...") is not an accessible live region; add role="alert" (or aria-live="assertive") to that container in app/ask-ai/page.tsx to announce IMAGE_QUALITY_LOW and other errors to assistive tech, and make the identical change in the error banner container inside the AskAIView component (components/AskAIView.tsx) so both UIs use a live region for screen readers.Source: Coding guidelines
app/public-rooms/page.tsx (1)
117-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLabel the filter inputs explicitly.
Both changed filter inputs rely on placeholder text alone. Add visible labels or wire them up with
aria-label/aria-labelledbyso the subject and tag filters are announced correctly to screen readers.As per coding guidelines,
**/*.tsxshould reviewAccessibility (aria labels, keyboard navigation).Also applies to: 136-152
🤖 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 `@app/public-rooms/page.tsx` around lines 117 - 126, The filter input currently relies on placeholder text (the input bound to customFilter via setCustomFilter and triggering fetchDoubts on Enter) and needs an accessible label; add a visible <label> or an aria attribute (e.g., aria-label="Subject filter" or aria-labelledby referencing a label id) to this input so screen readers announce it, and do the same for the other filter input used around the tag filter (the other input that also triggers fetchDoubts) to ensure both subject and tag filters are properly identified to assistive tech.Source: Coding guidelines
app/rooms/[id]/page.tsx (1)
119-160:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t let a hidden tag filter leak into the Ask AI tab.
tagFilteris shared across tabs, andfetchScopedDoubtsapplies it to every request. If a user types a tag in the community or teacher view and then switches toask-ai, the AI history is still filtered even though that tab exposes no tag-filter UI. Scope this filter per tab, or ignore/clear it outside the tabs that render it.🤖 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 `@app/rooms/`[id]/page.tsx around lines 119 - 160, fetchScopedDoubts currently applies the shared tagFilter to every request, causing the Ask AI tab (activeTab 'ai') to be unexpectedly filtered; update the logic so the tag is only sent for tabs that support it (e.g., 'community' and 'teacher') by either: 1) changing fetchScopedDoubts to accept an explicit tag argument and only append tag when that argument is non-empty and the activeTab is not 'ai', or 2) in the useEffect, pass an empty tag / call fetchScopedDoubts without tag when activeTab === 'ai'; adjust references to tagFilter, fetchScopedDoubts, activeTab and tabCache accordingly so AI history is never filtered by tag.app/api/doubts/route.ts (1)
188-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce classroom membership before creating classroom-scoped doubts.
This path trusts
classroomIdfrom the request body and persists it directly, but there is no membership/teacher check before writing the doubt and its tags. A signed-in user can post into any classroom by guessing an id.As per coding guidelines,
app/api/**should reviewMissing authentication/authorization checks.🤖 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 `@app/api/doubts/route.ts` around lines 188 - 215, The request currently trusts classroomId and writes a classroom-scoped doubt without authorization; before inserting (before the db.insert(doubtsTable) call that uses parsedClassroomId) query the classrooms/memberships table to verify the signed-in user (use the same identifier used elsewhere, e.g., email or user id) is a member or teacher of that classroom, and if not return a 403 error; only allow persisting classroomId and adding related tags when that membership check passes, otherwise strip classroomId or reject the request. Ensure you reference parsedClassroomId and the db insert into doubtsTable (and any subsequent tag insert logic) when adding this guard.Source: Coding guidelines
🤖 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 `@app/api/ask-ai/route.ts`:
- Around line 43-46: The current isUnclearVisionReply function wrongly treats
any short reply as unreadable due to normalizedReply.length < 50; remove that
hard length cutoff and instead detect truly empty/uninformative replies by
checking for meaningful content (e.g., require a minimum count of alphanumeric
characters or words, and reject only if the reply is composed mostly of
punctuation/whitespace or matches UNCLEAR_IMAGE_PATTERNS). Update
isUnclearVisionReply to drop the length check and replace it with a regex-based
content test (for example count of /\w/ chars or word count < 2) together with
the existing UNCLEAR_IMAGE_PATTERNS checks so concise but valid OCR results are
not misclassified.
In `@app/api/doubts/action/`[id]/route.ts:
- Around line 121-161: The update currently mutates the doubt (db.update on
doubtsTable) then deletes and recreates tag mappings (doubtTagsTable, tagsTable)
without a transaction, so a failure midway can leave the doubt updated but tags
lost; fix by wrapping the entire edit flow — the db.update(...) call that
produces updated, the db.delete(doubtTagsTable) and the loop that inserts/looks
up tags (tagsTable, doubtTagsTable) — inside a single database transaction (use
your project's db.transaction/runInTransaction API) so any error rolls back all
changes and only commits when all tag insertions and mappings succeed; ensure
the final NextResponse.json returns the updated row from the committed
transaction.
In `@app/api/doubts/route.ts`:
- Around line 207-247: The insert of the doubt (newDoubt) happens before tag
lookups/inserts and can leave partial state if later tag operations fail; wrap
the entire flow — creating the doubt, computing normalizedTags,
selecting/inserting into tagsTable, and inserting into doubtTagsTable — inside a
single db.transaction so all DB operations are atomic; use the transaction
client (e.g., tx) in place of db for tx.insert(doubtsTable).returning(),
tx.select(...).from(tagsTable).where(...), tx.insert(tagsTable).returning(), and
tx.insert(doubtTagsTable).onConflictDoNothing() so the transaction rolls back on
error and you still return the assembled savedTags with the new doubt on
success.
In `@app/api/tags/route.ts`:
- Around line 64-73: Validate the incoming payload before normalization/auth/DB
logic: replace the ad-hoc parseInt of rawClassroomId in the POST handler in
route.ts with a Zod schema that requires name as string and classroomId as an
optional integer or null, parse/validate req.json() against that schema and
return NextResponse.json({ error: ... }, { status: 400 }) on failure; only after
successful validation call normalizeTagName(name), compute a safe classroomId
number (guaranteed integer or null), then perform
canAccessClassroom(classroomId, email) and any tagsTable lookups/inserts to
avoid NaN being written to tagsTable.classroomId or skipping the classroom auth
guard.
In `@app/rooms/`[id]/page.tsx:
- Around line 335-351: The tag filter text inputs (the input bound to tagFilter
via value={tagFilter} and onChange={e => setTagFilter(e.target.value)} and the
second identical input further down) lack accessible labeling; add either a
visible <label> associated with the input or an aria-label/aria-labelledby
attribute (e.g., aria-label="Filter tag" or aria-labelledby referencing a nearby
label element) so screen readers can identify the control, and ensure the Clear
button remains reachable via keyboard; update both instances (the input using
tagFilter/setTagFilter at the shown block and the similar input around lines
445-461) to use the same accessible labeling approach.
In `@components/AskDoubt.tsx`:
- Around line 68-80: The useEffect inside the AskDoubt component only resets
subject, tags, and subjectWasEdited when doubtToEdit is falsy, leaving other
draft fields (content, imageUrl, fileName, tagDraft, suggestedSubject)
populated; update the else branch of the useEffect (watching defaultSubject and
doubtToEdit) to also clear setContent(""), setImageUrl(""), setFileName(""),
setTagDraft([] or ""), and setSuggestedSubject("") (or null) so that reopening
create mode always presents a blank draft, while preserving the existing
setSubject(defaultSubject), setTags([]) and setSubjectWasEdited(false) calls.
- Around line 258-307: Add programmatic accessibility: give the tag input a
persistent label linked via htmlFor/id (or an aria-label/aria-labelledby) so
screen readers can identify it (referencing the input using tagDraft and addTag
handlers), and add descriptive aria-labels to each remove button in the tags.map
(the buttons that call setTags to filter out a tag) such as "Remove tag {tag}"
so each chip's remove action is announced; also ensure the Suggest button
(addSuggestedTags) remains keyboard accessible and include an aria-label if its
visual text isn't sufficient for screen readers.
In `@configs/schema.ts`:
- Around line 155-164: The doubt_tags join table (doubtTagsTable) lacks foreign
key constraints for doubtId and tagId; add FK references so doubtId references
doubts.id and tagId references tags.id with ON DELETE CASCADE and ON UPDATE
CASCADE (or equivalent cascade semantics supported by your schema builder) and
ensure the constraint names are unique; update the pgTable definition for
doubtTagsTable to include these foreign key rules alongside the existing indexes
and uniqueDoubtTag to prevent orphaned rows when a doubt or tag is removed or
updated.
- Line 149: The current normalizedNameIndex (uniqueIndex "tag_scope_name_idx" on
table.normalizedName, table.classroomId) doesn't prevent duplicate public tags
because PostgreSQL treats NULLs as distinct; replace it with two partial unique
indexes: one unique index on table.normalizedName WHERE table.classroomId IS
NULL (to enforce one public tag per normalizedName) and another unique index on
(table.normalizedName, table.classroomId) WHERE table.classroomId IS NOT NULL
(to enforce uniqueness within classrooms); alternatively, change the schema to
store a non-null scope key and keep a single unique index on (normalizedName,
scopeKey).
In `@drizzle/0001_add_doubt_tags.sql`:
- Around line 13-22: The doubt_tags table currently lacks foreign key
constraints; update the CREATE TABLE "doubt_tags" statement to add FOREIGN KEY
constraints for "doubtId" referencing "doubts(id)" and for "tagId" referencing
"tags(id)", and add ON DELETE CASCADE (or appropriate cascade behavior) so that
deletions of rows in "doubts" or "tags" automatically remove related rows in
"doubt_tags"; ensure the constraints are named (e.g., doubt_tags_doubtId_fkey,
doubt_tags_tagId_fkey) or use inline FOREIGN KEY definitions and keep the
existing indexes and unique index ("doubt_tag_unique_idx") intact.
---
Outside diff comments:
In `@app/api/ask-ai/route.ts`:
- Around line 109-123: Add Zod validation for the incoming body before using
req.json() values: define a Zod schema that validates prompt (optional string),
type (enum 'standard' | ... default 'standard'), imageBase64 (optional string),
classroomId (optional string), and history (array of objects with role:
'user'|'assistant' and content: string). Replace the direct destructure of
req.json() with zodParse/tryParse on the parsed body and return
NextResponse.json({ error: validation.errors }, { status: 400 }) on failure;
then use the validated values for subsequent calls (moderateContent,
handleModerationViolation, messages.push(...history), DB/model calls) to prevent
malformed payloads from reaching those paths.
- Around line 89-118: Add a rate-limiting check at the very start of the POST
handler (before calling moderateContent, Groq queries, or any upstream model
attempts) to throttle by user (email) and fallback to IP; if the caller exceeds
their quota return a 429 JSON response. Implement this by invoking your existing
rate limiter utility (or add a token-bucket/redis-limiter) using the user email
(from email) or request IP as the key, and short-circuit the request inside POST
when the check fails; ensure the check happens before calls to moderateContent
and any functions that perform Groq/upstream model calls so abusive clients
cannot drive moderation or model quota.
In `@app/api/doubts/route.ts`:
- Around line 188-215: The request currently trusts classroomId and writes a
classroom-scoped doubt without authorization; before inserting (before the
db.insert(doubtsTable) call that uses parsedClassroomId) query the
classrooms/memberships table to verify the signed-in user (use the same
identifier used elsewhere, e.g., email or user id) is a member or teacher of
that classroom, and if not return a 403 error; only allow persisting classroomId
and adding related tags when that membership check passes, otherwise strip
classroomId or reject the request. Ensure you reference parsedClassroomId and
the db insert into doubtsTable (and any subsequent tag insert logic) when adding
this guard.
In `@app/ask-ai/page.tsx`:
- Around line 379-428: The error banner currently rendered when errorMsg is set
(the div with className "flex items-start gap-3 p-4 bg-red-500/10 ...") is not
an accessible live region; add role="alert" (or aria-live="assertive") to that
container in app/ask-ai/page.tsx to announce IMAGE_QUALITY_LOW and other errors
to assistive tech, and make the identical change in the error banner container
inside the AskAIView component (components/AskAIView.tsx) so both UIs use a live
region for screen readers.
In `@app/public-rooms/page.tsx`:
- Around line 117-126: The filter input currently relies on placeholder text
(the input bound to customFilter via setCustomFilter and triggering fetchDoubts
on Enter) and needs an accessible label; add a visible <label> or an aria
attribute (e.g., aria-label="Subject filter" or aria-labelledby referencing a
label id) to this input so screen readers announce it, and do the same for the
other filter input used around the tag filter (the other input that also
triggers fetchDoubts) to ensure both subject and tag filters are properly
identified to assistive tech.
In `@app/rooms/`[id]/page.tsx:
- Around line 119-160: fetchScopedDoubts currently applies the shared tagFilter
to every request, causing the Ask AI tab (activeTab 'ai') to be unexpectedly
filtered; update the logic so the tag is only sent for tabs that support it
(e.g., 'community' and 'teacher') by either: 1) changing fetchScopedDoubts to
accept an explicit tag argument and only append tag when that argument is
non-empty and the activeTab is not 'ai', or 2) in the useEffect, pass an empty
tag / call fetchScopedDoubts without tag when activeTab === 'ai'; adjust
references to tagFilter, fetchScopedDoubts, activeTab and tabCache accordingly
so AI history is never filtered by tag.
🪄 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: 692862cd-626e-4982-8e97-166a2bb1c868
📒 Files selected for processing (16)
app/api/ask-ai/route.tsapp/api/doubts/[id]/pin/route.tsapp/api/doubts/action/[id]/route.tsapp/api/doubts/route.tsapp/api/tags/route.tsapp/ask-ai/page.tsxapp/bookmarks/page.tsxapp/not-found.tsxapp/public-rooms/page.tsxapp/rooms/[id]/page.tsxcomponents/AskAIView.tsxcomponents/AskDoubt.tsxcomponents/DoubtCard.tsxcomponents/DoubtRepliesModal.tsxconfigs/schema.tsdrizzle/0001_add_doubt_tags.sql
Comment on lines
+43
to
+46
| function isUnclearVisionReply(reply: string) { | ||
| const normalizedReply = reply.replace(/^SUBJECT:\s*.+\n?/im, '').trim(); | ||
|
|
||
| return normalizedReply.length < 50 || UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply)); |
Contributor
There was a problem hiding this comment.
Don't classify every short vision reply as unreadable.
normalizedReply.length < 50 will reject valid image solves that happen to be concise, like a one-line transcription or a short final answer. That turns successful OCR into a hard 422 IMAGE_QUALITY_LOW and blocks the user from seeing a correct result.
💡 Safer classification sketch
function isUnclearVisionReply(reply: string) {
const normalizedReply = reply.replace(/^SUBJECT:\s*.+\n?/im, '').trim();
-
- return normalizedReply.length < 50 || UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply));
+ const looksStructured =
+ /##\s+Step-by-step explanation/i.test(normalizedReply) ||
+ /##\s+Final Answer/i.test(normalizedReply);
+
+ return !looksStructured &&
+ UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply));
}📝 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.
Suggested change
| function isUnclearVisionReply(reply: string) { | |
| const normalizedReply = reply.replace(/^SUBJECT:\s*.+\n?/im, '').trim(); | |
| return normalizedReply.length < 50 || UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply)); | |
| function isUnclearVisionReply(reply: string) { | |
| const normalizedReply = reply.replace(/^SUBJECT:\s*.+\n?/im, '').trim(); | |
| const looksStructured = | |
| /##\s+Step-by-step explanation/i.test(normalizedReply) || | |
| /##\s+Final Answer/i.test(normalizedReply); | |
| return !looksStructured && | |
| UNCLEAR_IMAGE_PATTERNS.some((pattern) => pattern.test(normalizedReply)); | |
| } |
🤖 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 `@app/api/ask-ai/route.ts` around lines 43 - 46, The current
isUnclearVisionReply function wrongly treats any short reply as unreadable due
to normalizedReply.length < 50; remove that hard length cutoff and instead
detect truly empty/uninformative replies by checking for meaningful content
(e.g., require a minimum count of alphanumeric characters or words, and reject
only if the reply is composed mostly of punctuation/whitespace or matches
UNCLEAR_IMAGE_PATTERNS). Update isUnclearVisionReply to drop the length check
and replace it with a regex-based content test (for example count of /\w/ chars
or word count < 2) together with the existing UNCLEAR_IMAGE_PATTERNS checks so
concise but valid OCR results are not misclassified.
Comment on lines
+121
to
+161
| const [updated] = await db.update(doubtsTable) | ||
| .set({ | ||
| content: content || null, | ||
| subject, | ||
| imageUrl: imageUrl || null | ||
| }) | ||
| .where(eq(doubtsTable.id, doubtId)) | ||
| .returning(); | ||
| return NextResponse.json(updated[0]); | ||
|
|
||
| const normalizedTags: string[] = Array.from(new Set( | ||
| (Array.isArray(tags) ? tags : []) | ||
| .map((tag: string) => tag.trim().replace(/\s+/g, " ").toLowerCase()) | ||
| .filter(Boolean) | ||
| )).slice(0, 8); | ||
|
|
||
| await db.delete(doubtTagsTable).where(eq(doubtTagsTable.doubtId, doubtId)); | ||
|
|
||
| const savedTags: any[] = []; | ||
| for (const normalizedName of normalizedTags) { | ||
| const [existingTag] = await db.select().from(tagsTable).where(and( | ||
| eq(tagsTable.normalizedName, normalizedName), | ||
| doubt.classroomId ? eq(tagsTable.classroomId, doubt.classroomId) : isNull(tagsTable.classroomId) | ||
| )).limit(1); | ||
|
|
||
| const [tagRecord] = existingTag | ||
| ? [existingTag] | ||
| : await db.insert(tagsTable).values({ | ||
| name: normalizedName.replace(/\b\w/g, (char) => char.toUpperCase()), | ||
| normalizedName, | ||
| classroomId: doubt.classroomId, | ||
| createdByEmail: email || null, | ||
| }).returning(); | ||
|
|
||
| savedTags.push(tagRecord); | ||
| await db.insert(doubtTagsTable).values({ | ||
| doubtId, | ||
| tagId: tagRecord.id, | ||
| }).onConflictDoNothing(); | ||
| } | ||
|
|
||
| return NextResponse.json({ ...updated, tags: savedTags }); |
Contributor
There was a problem hiding this comment.
The edit flow can permanently drop tags on partial failure.
This sequence updates the doubt, deletes all existing tag mappings, and then recreates them without a transaction. Any failure after Line 136 leaves the edit committed but the old tags gone. The doubt update and tag replacement need to run atomically.
🤖 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 `@app/api/doubts/action/`[id]/route.ts around lines 121 - 161, The update
currently mutates the doubt (db.update on doubtsTable) then deletes and
recreates tag mappings (doubtTagsTable, tagsTable) without a transaction, so a
failure midway can leave the doubt updated but tags lost; fix by wrapping the
entire edit flow — the db.update(...) call that produces updated, the
db.delete(doubtTagsTable) and the loop that inserts/looks up tags (tagsTable,
doubtTagsTable) — inside a single database transaction (use your project's
db.transaction/runInTransaction API) so any error rolls back all changes and
only commits when all tag insertions and mappings succeed; ensure the final
NextResponse.json returns the updated row from the committed transaction.
Comment on lines
+207
to
+247
| const [newDoubt] = await db.insert(doubtsTable).values({ | ||
| userName, | ||
| userEmail: email, | ||
| subject, | ||
| subTopic, | ||
| content, | ||
| imageUrl, | ||
| classroomId: classroomId ? parseInt(classroomId.toString()) : null, | ||
| classroomId: parsedClassroomId, | ||
| type | ||
| }).returning(); | ||
|
|
||
| return NextResponse.json(newDoubt[0]); | ||
| const normalizedTags: string[] = Array.from(new Set( | ||
| (Array.isArray(tags) ? tags : []) | ||
| .map((tag: string) => tag.trim().replace(/\s+/g, " ").toLowerCase()) | ||
| .filter(Boolean) | ||
| )).slice(0, 8); | ||
|
|
||
| const savedTags: any[] = []; | ||
| for (const normalizedName of normalizedTags) { | ||
| const [existingTag] = await db.select().from(tagsTable).where(and( | ||
| eq(tagsTable.normalizedName, normalizedName), | ||
| parsedClassroomId ? eq(tagsTable.classroomId, parsedClassroomId) : isNull(tagsTable.classroomId) | ||
| )).limit(1); | ||
|
|
||
| const [tagRecord] = existingTag | ||
| ? [existingTag] | ||
| : await db.insert(tagsTable).values({ | ||
| name: normalizedName.replace(/\b\w/g, (char) => char.toUpperCase()), | ||
| normalizedName, | ||
| classroomId: parsedClassroomId, | ||
| createdByEmail: email, | ||
| }).returning(); | ||
|
|
||
| savedTags.push(tagRecord); | ||
| await db.insert(doubtTagsTable).values({ | ||
| doubtId: newDoubt.id, | ||
| tagId: tagRecord.id, | ||
| }).onConflictDoNothing(); | ||
| } | ||
|
|
||
| return NextResponse.json({ ...newDoubt, tags: savedTags }); |
Contributor
There was a problem hiding this comment.
Wrap doubt creation and tag persistence in one transaction.
newDoubt is inserted before any of the tag lookups/inserts/mappings run. If one of those later steps fails, the request returns 500 but the doubt row is already committed without the requested tag set. This is one logical write across three tables now, so it should execute inside db.transaction(...).
🤖 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 `@app/api/doubts/route.ts` around lines 207 - 247, The insert of the doubt
(newDoubt) happens before tag lookups/inserts and can leave partial state if
later tag operations fail; wrap the entire flow — creating the doubt, computing
normalizedTags, selecting/inserting into tagsTable, and inserting into
doubtTagsTable — inside a single db.transaction so all DB operations are atomic;
use the transaction client (e.g., tx) in place of db for
tx.insert(doubtsTable).returning(), tx.select(...).from(tagsTable).where(...),
tx.insert(tagsTable).returning(), and
tx.insert(doubtTagsTable).onConflictDoNothing() so the transaction rolls back on
error and you still return the assembled savedTags with the new doubt on
success.
Comment on lines
+64
to
+73
| const { name, classroomId: rawClassroomId } = await req.json(); | ||
| const normalizedName = normalizeTagName(name || ""); | ||
| const classroomId = rawClassroomId ? parseInt(rawClassroomId.toString()) : null; | ||
|
|
||
| if (!normalizedName) { | ||
| return NextResponse.json({ error: "Tag name is required" }, { status: 400 }); | ||
| } | ||
|
|
||
| if (classroomId && !(await canAccessClassroom(classroomId, email))) { | ||
| return NextResponse.json({ error: "Access denied to this classroom" }, { status: 403 }); |
Contributor
There was a problem hiding this comment.
Reject invalid { name, classroomId } payloads before auth checks and DB writes
rawClassroomId is coerced via parseInt(...) and then used in if (classroomId && !(await canAccessClassroom(...))); when parsing fails (e.g., "abc" → NaN), classroomId becomes falsy so the classroom auth guard is skipped, and the “existing tag” lookup falls back to the isNull(tagsTable.classroomId) branch. The insert then writes classroomId: NaN into tagsTable.classroomId (an integer() column), which will typically error and surface as a 500 instead of a 400. Validate { name, classroomId } up front with Zod (e.g., name as string, classroomId as integer or null) and return 400 for invalid payloads before calling normalizeTagName / performing any DB writes.
🤖 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 `@app/api/tags/route.ts` around lines 64 - 73, Validate the incoming payload
before normalization/auth/DB logic: replace the ad-hoc parseInt of
rawClassroomId in the POST handler in route.ts with a Zod schema that requires
name as string and classroomId as an optional integer or null, parse/validate
req.json() against that schema and return NextResponse.json({ error: ... }, {
status: 400 }) on failure; only after successful validation call
normalizeTagName(name), compute a safe classroomId number (guaranteed integer or
null), then perform canAccessClassroom(classroomId, email) and any tagsTable
lookups/inserts to avoid NaN being written to tagsTable.classroomId or skipping
the classroom auth guard.
Comment on lines
+335
to
+351
| <div className="flex items-center gap-2"> | ||
| <input | ||
| type="text" | ||
| value={tagFilter} | ||
| onChange={(e) => setTagFilter(e.target.value)} | ||
| placeholder="Filter tag" | ||
| className="w-32 bg-slate-950/50 border border-white/10 rounded-xl px-3 py-2.5 text-[10px] font-bold text-white placeholder:text-slate-600 focus:outline-none focus:border-blue-500/50" | ||
| /> | ||
| {tagFilter && ( | ||
| <button | ||
| onClick={() => setTagFilter("")} | ||
| className="text-[9px] font-black uppercase tracking-widest text-slate-500 hover:text-white" | ||
| > | ||
| Clear | ||
| </button> | ||
| )} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Label the classroom tag-filter inputs.
Both new tag-filter inputs rely on placeholder text only. Add a visible label or aria-label/aria-labelledby so these controls are usable with assistive technology.
As per coding guidelines, **/*.tsx should review Accessibility (aria labels, keyboard navigation).
Also applies to: 445-461
🤖 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 `@app/rooms/`[id]/page.tsx around lines 335 - 351, The tag filter text inputs
(the input bound to tagFilter via value={tagFilter} and onChange={e =>
setTagFilter(e.target.value)} and the second identical input further down) lack
accessible labeling; add either a visible <label> associated with the input or
an aria-label/aria-labelledby attribute (e.g., aria-label="Filter tag" or
aria-labelledby referencing a nearby label element) so screen readers can
identify the control, and ensure the Clear button remains reachable via
keyboard; update both instances (the input using tagFilter/setTagFilter at the
shown block and the similar input around lines 445-461) to use the same
accessible labeling approach.
Source: Coding guidelines
Comment on lines
68
to
80
| useEffect(() => { | ||
| if (doubtToEdit) { | ||
| setContent(doubtToEdit.content || ""); | ||
| setSubject(doubtToEdit.subject || defaultSubject); | ||
| setImageUrl(doubtToEdit.imageUrl || ""); | ||
| setFileName(doubtToEdit.imageUrl ? "Existing Image" : ""); | ||
| setTags(doubtToEdit.tags?.map((tag: any) => tag.name) || []); | ||
| } else { | ||
| setSubject(defaultSubject); | ||
| setTags([]); | ||
| setSubjectWasEdited(false); | ||
| } | ||
| }, [defaultSubject, doubtToEdit]); |
Contributor
There was a problem hiding this comment.
Reset the full draft when reopening create mode.
In create mode this effect only clears subject, tags, and subjectWasEdited. If the modal is closed and reopened with the same defaultSubject, the previous content, imageUrl, fileName, tagDraft, and suggestedSubject stick around, which makes accidental resubmission easy.
🤖 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 `@components/AskDoubt.tsx` around lines 68 - 80, The useEffect inside the
AskDoubt component only resets subject, tags, and subjectWasEdited when
doubtToEdit is falsy, leaving other draft fields (content, imageUrl, fileName,
tagDraft, suggestedSubject) populated; update the else branch of the useEffect
(watching defaultSubject and doubtToEdit) to also clear setContent(""),
setImageUrl(""), setFileName(""), setTagDraft([] or ""), and
setSuggestedSubject("") (or null) so that reopening create mode always presents
a blank draft, while preserving the existing setSubject(defaultSubject),
setTags([]) and setSubjectWasEdited(false) calls.
Comment on lines
+258
to
+307
| <div className="space-y-3"> | ||
| <div className="flex items-center justify-between gap-3 px-1"> | ||
| <label className="text-[10px] font-black uppercase tracking-[0.25em] text-slate-500">Tags</label> | ||
| <button | ||
| type="button" | ||
| onClick={addSuggestedTags} | ||
| disabled={!content.trim()} | ||
| className="flex items-center gap-1.5 text-[9px] font-black uppercase tracking-widest text-blue-400 hover:text-blue-300 disabled:opacity-40" | ||
| > | ||
| <Sparkles className="w-3 h-3" /> Suggest | ||
| </button> | ||
| </div> | ||
| <div className="flex items-center gap-2 bg-white/5 border border-white/10 rounded-2xl px-4 py-3 focus-within:border-blue-500/50"> | ||
| <Tags className="w-4 h-4 text-slate-500" /> | ||
| <input | ||
| type="text" | ||
| value={tagDraft} | ||
| onChange={(e) => setTagDraft(e.target.value)} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === ",") { | ||
| e.preventDefault(); | ||
| addTag(tagDraft); | ||
| } | ||
| }} | ||
| placeholder="Add a tag and press Enter" | ||
| className="flex-1 bg-transparent text-sm text-white placeholder:text-slate-600 focus:outline-none" | ||
| /> | ||
| <button | ||
| type="button" | ||
| onClick={() => addTag(tagDraft)} | ||
| className="text-[9px] font-black uppercase tracking-widest text-slate-400 hover:text-white" | ||
| > | ||
| Add | ||
| </button> | ||
| </div> | ||
| {tags.length > 0 && ( | ||
| <div className="flex flex-wrap gap-2"> | ||
| {tags.map((tag) => ( | ||
| <button | ||
| key={tag} | ||
| type="button" | ||
| onClick={() => setTags((currentTags) => currentTags.filter((item) => item !== tag))} | ||
| className="flex items-center gap-2 px-3 py-1.5 rounded-full bg-blue-500/10 border border-blue-500/20 text-blue-200 text-[10px] font-bold" | ||
| > | ||
| {tag} | ||
| <X className="w-3 h-3" /> | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} |
Contributor
There was a problem hiding this comment.
Add accessible labels to the new tag controls.
The tag input and remove-tag buttons are only discoverable through placeholder/chip text right now. Add an explicit programmatic label for the input and aria-labels for the remove actions so screen-reader users can add and remove tags reliably.
As per coding guidelines, **/*.tsx should review Accessibility (aria labels, keyboard navigation).
🤖 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 `@components/AskDoubt.tsx` around lines 258 - 307, Add programmatic
accessibility: give the tag input a persistent label linked via htmlFor/id (or
an aria-label/aria-labelledby) so screen readers can identify it (referencing
the input using tagDraft and addTag handlers), and add descriptive aria-labels
to each remove button in the tags.map (the buttons that call setTags to filter
out a tag) such as "Remove tag {tag}" so each chip's remove action is announced;
also ensure the Suggest button (addSuggestedTags) remains keyboard accessible
and include an aria-label if its visual text isn't sufficient for screen
readers.
Source: Coding guidelines
| createdAt: timestamp().defaultNow().notNull(), | ||
| }, (table) => ({ | ||
| classroomIdIndex: index("tag_classroomId_idx").on(table.classroomId), | ||
| normalizedNameIndex: uniqueIndex("tag_scope_name_idx").on(table.normalizedName, table.classroomId), |
Contributor
There was a problem hiding this comment.
tag_scope_name_idx does not enforce uniqueness for public tags.
PostgreSQL unique indexes treat NULL values as distinct, so (normalizedName, classroomId) still allows multiple public rows with the same normalizedName when classroomId is NULL. The new API flows assume one reusable public tag per normalized name, so duplicates here will make reuse and filtering nondeterministic. Use separate partial unique indexes for classroomId IS NULL and classroomId IS NOT NULL, or store a non-null scope key instead.
As per coding guidelines, configs/schema.ts should review Proper indexes on frequently queried columns.
🤖 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 `@configs/schema.ts` at line 149, The current normalizedNameIndex (uniqueIndex
"tag_scope_name_idx" on table.normalizedName, table.classroomId) doesn't prevent
duplicate public tags because PostgreSQL treats NULLs as distinct; replace it
with two partial unique indexes: one unique index on table.normalizedName WHERE
table.classroomId IS NULL (to enforce one public tag per normalizedName) and
another unique index on (table.normalizedName, table.classroomId) WHERE
table.classroomId IS NOT NULL (to enforce uniqueness within classrooms);
alternatively, change the schema to store a non-null scope key and keep a single
unique index on (normalizedName, scopeKey).
Source: Coding guidelines
Comment on lines
+155
to
+164
| export const doubtTagsTable = pgTable("doubt_tags", { | ||
| id: integer().primaryKey().generatedAlwaysAsIdentity(), | ||
| doubtId: integer().notNull(), | ||
| tagId: integer().notNull(), | ||
| createdAt: timestamp().defaultNow().notNull(), | ||
| }, (table) => ({ | ||
| doubtIdIndex: index("doubt_tag_doubtId_idx").on(table.doubtId), | ||
| tagIdIndex: index("doubt_tag_tagId_idx").on(table.tagId), | ||
| uniqueDoubtTag: uniqueIndex("doubt_tag_unique_idx").on(table.doubtId, table.tagId), | ||
| })); |
Contributor
There was a problem hiding this comment.
Add foreign keys and cascade rules to doubt_tags.
This join table currently has no foreign key back to doubts or tags. Deleting a doubt or tag will leave orphaned mappings behind, and the new tag hydration/filter queries will keep scanning those dead rows. doubtId should reference doubts.id and tagId should reference tags.id, both with cascade delete/update semantics.
As per coding guidelines, configs/schema.ts should review Missing foreign key constraints and Cascade delete/update rules.
🤖 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 `@configs/schema.ts` around lines 155 - 164, The doubt_tags join table
(doubtTagsTable) lacks foreign key constraints for doubtId and tagId; add FK
references so doubtId references doubts.id and tagId references tags.id with ON
DELETE CASCADE and ON UPDATE CASCADE (or equivalent cascade semantics supported
by your schema builder) and ensure the constraint names are unique; update the
pgTable definition for doubtTagsTable to include these foreign key rules
alongside the existing indexes and uniqueDoubtTag to prevent orphaned rows when
a doubt or tag is removed or updated.
Source: Coding guidelines
Comment on lines
+13
to
+22
| CREATE TABLE IF NOT EXISTS "doubt_tags" ( | ||
| "id" integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | ||
| "doubtId" integer NOT NULL, | ||
| "tagId" integer NOT NULL, | ||
| "createdAt" timestamp DEFAULT now() NOT NULL | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS "doubt_tag_doubtId_idx" ON "doubt_tags" ("doubtId"); | ||
| CREATE INDEX IF NOT EXISTS "doubt_tag_tagId_idx" ON "doubt_tags" ("tagId"); | ||
| CREATE UNIQUE INDEX IF NOT EXISTS "doubt_tag_unique_idx" ON "doubt_tags" ("doubtId", "tagId"); |
Contributor
There was a problem hiding this comment.
Add missing foreign key constraints to doubt_tags table.
The doubt_tags table references doubtId and tagId without explicit FOREIGN KEY constraints. This breaks referential integrity, allowing orphaned rows to accumulate and risking data corruption.
Add constraints to reference the parent tables:
doubtId→doubts(id)tagId→tags(id)
Also define appropriate cascade delete rules so that when a doubt or tag is deleted, associated doubt_tags rows are cleaned up automatically.
🔐 Proposed fix: Add FOREIGN KEY constraints with CASCADE DELETE
CREATE TABLE IF NOT EXISTS "doubt_tags" (
"id" integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
"doubtId" integer NOT NULL,
"tagId" integer NOT NULL,
"createdAt" timestamp DEFAULT now() NOT NULL
+ FOREIGN KEY ("doubtId") REFERENCES "doubts"("id") ON DELETE CASCADE,
+ FOREIGN KEY ("tagId") REFERENCES "tags"("id") ON DELETE CASCADE
);📝 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.
Suggested change
| CREATE TABLE IF NOT EXISTS "doubt_tags" ( | |
| "id" integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | |
| "doubtId" integer NOT NULL, | |
| "tagId" integer NOT NULL, | |
| "createdAt" timestamp DEFAULT now() NOT NULL | |
| ); | |
| CREATE INDEX IF NOT EXISTS "doubt_tag_doubtId_idx" ON "doubt_tags" ("doubtId"); | |
| CREATE INDEX IF NOT EXISTS "doubt_tag_tagId_idx" ON "doubt_tags" ("tagId"); | |
| CREATE UNIQUE INDEX IF NOT EXISTS "doubt_tag_unique_idx" ON "doubt_tags" ("doubtId", "tagId"); | |
| CREATE TABLE IF NOT EXISTS "doubt_tags" ( | |
| "id" integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | |
| "doubtId" integer NOT NULL, | |
| "tagId" integer NOT NULL, | |
| "createdAt" timestamp DEFAULT now() NOT NULL, | |
| FOREIGN KEY ("doubtId") REFERENCES "doubts"("id") ON DELETE CASCADE, | |
| FOREIGN KEY ("tagId") REFERENCES "tags"("id") ON DELETE CASCADE | |
| ); | |
| CREATE INDEX IF NOT EXISTS "doubt_tag_doubtId_idx" ON "doubt_tags" ("doubtId"); | |
| CREATE INDEX IF NOT EXISTS "doubt_tag_tagId_idx" ON "doubt_tags" ("tagId"); | |
| CREATE UNIQUE INDEX IF NOT EXISTS "doubt_tag_unique_idx" ON "doubt_tags" ("doubtId", "tagId"); |
🤖 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 `@drizzle/0001_add_doubt_tags.sql` around lines 13 - 22, The doubt_tags table
currently lacks foreign key constraints; update the CREATE TABLE "doubt_tags"
statement to add FOREIGN KEY constraints for "doubtId" referencing "doubts(id)"
and for "tagId" referencing "tags(id)", and add ON DELETE CASCADE (or
appropriate cascade behavior) so that deletions of rows in "doubts" or "tags"
automatically remove related rows in "doubt_tags"; ensure the constraints are
named (e.g., doubt_tags_doubtId_fkey, doubt_tags_tagId_fkey) or use inline
FOREIGN KEY definitions and keep the existing indexes and unique index
("doubt_tag_unique_idx") intact.
Source: Coding guidelines
Owner
|
Hi @saurabhhhcodes! Thanks for submitting this PR. Here is a detailed review of your changes: 🤖 Bot Review Feedback to ResolveCodeRabbit and CodeAnt have highlighted some important issues that need to be addressed. Please prioritize fixing these:
Detailsapp/api/doubts/route.ts (Line 222):Suggestion: The tags normalization assumes every element is a string and calls trim() directly; a crafted payload with non-string entries (number/object/null) will throw at runtime and return 500. Guard with a type check before trimming or coerce safely. [type error]Sev...
File File drizzle/0001_add_doubt_tags.sql (Line 11):Suggestion: The unique index on ( normalizedName, classroomId) does not prevent duplicates when classroomId is NULL (PostgreSQL treats NULL values as distinct in unique indexes). This allows multiple identical public tags and breaks the intended "one tag per scope" rule. Use a separate ...Vercel |
ask-ai syntax/compilation issues on the main branch. Please pull/merge the latest main branch into your branch to get these fixes, which should resolve common test and Vercel build failures.
Owner
|
knoxiboy
added
merge-conflict
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
2 participants
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.
User description
Summary\n- add bookmarks, doubt pinning, and export flows\n- expand public room and ask-ai user workflows\n- keep the assigned work branch moving as one coherent PR\n\n## Validation\n- reviewed the large feature merge diff\n- no unrelated repository-wide refactor beyond the assigned scope
CodeAnt-AI Description
Add tags to doubts, tag filters, and clearer AI image error handling
What Changed
Impact
✅ Faster doubt tagging✅ Quicker finding of related questions✅ Clearer guidance after blurry image uploads💡 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
Release Notes
New Features
Bug Fixes