fix: enforce maximum query limit on public doubt board#780
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 · |
WalkthroughThe ChangesDoubts Query Limit Clamp
Estimated code review effort: 1 (Trivial) | ~3 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DoubtsRoute as GET /api/doubts
participant ParsePositiveInt
Client->>DoubtsRoute: request with limit query param
DoubtsRoute->>ParsePositiveInt: parsePositiveInt(limitStr, 20, 100)
ParsePositiveInt-->>ParsePositiveInt: validate, parse, clamp to max
ParsePositiveInt-->>DoubtsRoute: clamped limit value
DoubtsRoute-->>Client: doubts list bounded by limit
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
gssoc'26
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| if (!value || !/^d+$/.test(value.trim())) { | ||
| return fallback; |
There was a problem hiding this comment.
Suggestion: The numeric validation regex is misspelled as ^d+$, so normal digit strings like 10 never pass validation and the function always returns the fallback for valid numeric input. Replace it with a digit regex so valid query params are actually parsed. [typo]
Severity Level: Major ⚠️
- ❌ Doubt board pagination ignores user-specified limit values.
- ❌ Notifications pagination ignores page and offset parameters.
- ⚠️ Max limit enforcement relies on hardcoded defaults, not input.
- ⚠️ Client UX degraded; numeric query params have no effect.Steps of Reproduction ✅
1. The GET handler for notifications at `src/app/api/notifications/route.ts:8-26` reads
`limitStr`, `offsetStr`, and `pageStr` from the URL and calls `parsePositiveInt(limitStr,
50)` and `parsePositiveInt(pageStr, 1)` to derive pagination parameters.
2. The public doubt board GET handler at `src/app/api/doubts/route.ts:17-26` similarly
reads `pageStr`, `offsetStr`, and `limitStr` and calls `parsePositiveInt(limitStr, 20,
100)` and `parsePositiveInt(offsetStr, 0)` / `parsePositiveInt(pageStr, 1)` to compute
`limit`, `offset`, and `page`.
3. Both handlers use the shared utility `parsePositiveInt` implemented at
`src/lib/utils.ts:8-27`, which validates the string with `if (!value ||
!/^d+$/.test(value.trim())) { return fallback; }` (lines 13-14) before converting it with
`Number(value)`.
4. When a client calls either endpoint with a normal numeric query, e.g. `GET
/api/doubts?limit=10&page=3` or `GET /api/notifications?limit=25&page=2`, the string
`"10"` or `"25"` fails the regex `^d+$` at `src/lib/utils.ts:13` (since it matches only
repeated `'d'` characters, not digits), causing `parsePositiveInt` to immediately return
the fallback (20 or 50) instead of the requested value. As a result, user-supplied numeric
pagination parameters are silently ignored and replaced with defaults on every request.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/utils.ts
**Line:** 13:14
**Comment:**
*Typo: The numeric validation regex is misspelled as `^d+$`, so normal digit strings like `10` never pass validation and the function always returns the fallback for valid numeric input. Replace it with a digit regex so valid query params are actually parsed.
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. |
d5c19a3 to
5aa7a57
Compare
5aa7a57 to
80bbed2
Compare
|
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 · |
|
CodeAnt AI Incremental review completed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (12)
src/configs/schema.ts (2)
48-48: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueMembership
rolehas no DB-level constraint.
roleis a free-formvarchar(20)with a default of"member", but valid values (e.g.,owner,admin,teacher,member) are only enforced in application code (src/app/api/analytics/route.tsline 49 checks['owner', 'admin', 'teacher'].includes(...)). A DBcheckconstraint or Postgres enum would prevent invalid roles from being inserted directly or via future code paths that skip this app-level check.🤖 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/configs/schema.ts` at line 48, The Membership `role` field in the schema is only validated in application code, so add a DB-level constraint in the schema definition to restrict it to the allowed roles. Update the `role` column in `src/configs/schema.ts` (and any related migration/schema setup) to use either a Postgres enum or a check constraint so inserts through any path cannot store invalid values. Keep the existing default of member and ensure the constraint matches the role values used by the analytics route and other membership logic.
34-63: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider referencing
usersTable.idinstead ofusersTable.emailfor membership FK.
organizationMembershipsTable.userEmailis a valid FK target sinceusersTable.emailis unique, but referencing the surrogate primary key (usersTable.id) is more idiomatic and avoids issues if emails are ever updated (email changes would need cascading updates across FK-referencing tables). Also,organizationsTable.ownerEmailhas no FK constraint at all, so a stale/nonexistent owner email is possible.Not blocking — matches existing repo convention (e.g.
classroomsTable.teacherEmail) of loosely-typed email references without constraints.🤖 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/configs/schema.ts` around lines 34 - 63, Update the schema in organizationsTable and organizationMembershipsTable to use proper user references: change organizationMembershipsTable’s user FK to point at usersTable.id through a userId-style column if needed, rather than usersTable.email, and add a foreign key for organizationsTable.ownerEmail to the users table so the owner is constrained too. Keep the existing table definitions and unique constraints intact, and adjust any related column names or references in the schema accordingly.src/app/api/doubts/route.ts (2)
128-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
anytyping added across map/reduce callbacks.Same pattern as in
src/app/api/analytics/route.ts— these callbacks operate on already-typed Drizzle query results (userBookmarks,userLikes,tagRows,doubts), so casting toanyisn't necessary and loses compile-time field checking. Not blocking, but worth reverting where feasible for real typing hardening rather than typing suppression.Also applies to: 209-210, 222-223, 239-242, 252-252, 263-263
🤖 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/route.ts` at line 128, Remove the unnecessary any annotations from the map/reduce callbacks in the doubts route and rely on the inferred Drizzle result types instead. Update the callbacks around userBookmarks, userLikes, tagRows, and doubts to use the existing typed objects directly so fields like doubtId, userId, and tag data remain compile-time checked. Keep the logic unchanged in the route handlers and helper expressions, just restore proper typing in the affected callbacks and reducers.
139-145: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftLimit cap verified correct; offset remains unbounded.
The
limitcap of 100 correctly addresses the unbounded-query issue. However,offset(line 140-141) has nomaxand can still be arbitrarily large; a very largeOFFSETondoubtsTablecan itself be slow on Postgres/Neon even with a cappedLIMIT, since the DB must still scan/skip that many rows. Given the linked issue's concern is specifically about avoiding Neon's 5s timeout, consider also capping offset or moving toward the cursor/keyset pagination the issue mentions as a longer-term fix.🤖 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/route.ts` around lines 139 - 145, The pagination logic in route.ts still allows an unbounded OFFSET because `offset` is derived directly from `offsetStr` or `pageStr` without any maximum, so large queries can still hit Postgres/Neon timeouts even with `limit` capped. Update the pagination flow around `parsePositiveInt`, `offset`, and `page` to either enforce a maximum offset or switch the `doubtsTable` query to cursor/keyset pagination, keeping the existing `limit` cap as-is and ensuring the query cannot request arbitrarily large skips.src/app/api/analytics/route.ts (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWidespread
anytyping weakens type safety instead of hardening it.Drizzle's
db.select({...})already returns strongly-typed rows, so callbacks likeorgClassrooms.map((c: any) => c.id),userMemberships.map((m: any) => m.classroomId),mostAskedTopics.map((topic: any, index: any) => ...),weakTopics.filter((t: any) => ...), andtopContributors.map((c: any) => ...)don't needany. This discards compiler-checked field names/types on the very data the queries were just typed to return.♻️ Example fix
- activeClassroomIds = orgClassrooms.map((c: any) => c.id); + activeClassroomIds = orgClassrooms.map((c) => c.id);Also applies to: 70-70, 202-202, 267-268
🤖 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/analytics/route.ts` at line 58, Remove the unnecessary `any` annotations in `route.ts` callbacks so the typed query results from `db.select(...)` keep their inferred field types. Update the array helpers in `orgClassrooms.map`, `userMemberships.map`, `mostAskedTopics.map`, `weakTopics.filter`, and `topContributors.map` to use the inferred element types from those variables instead of `any`, keeping the same logic but allowing TypeScript to validate property access like `id`, `classroomId`, and `topic`.src/app/api/admin/moderation/route.ts (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer inferred/explicit types over
any.
flagsPerDay's shape is already known from the precedingdb.select({ date, count })query (Lines 35-38). Annotating the callback parameter asanyhere (and in the similar patterns across this cohort) discards that inference and the type-checking it provides, rather than just omitting the annotation to let TypeScript infer it, or usingtypeof flagsPerDay[number].🤖 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/admin/moderation/route.ts` around lines 40 - 43, The `flagsPerDay.map` callback is using `any`, which throws away the inferred shape from the preceding `db.select({ date, count })` query. Remove the `any` annotation on the callback parameter (or replace it with the inferred item type such as `typeof flagsPerDay[number]`) so the `formattedFlagsPerDay` mapping in `route.ts` keeps TypeScript’s type-checking while preserving the existing date/count transformation.src/app/api/rooms/join/route.ts (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer explicit string typing over
any.
allowedEmailDomainsis presumably astring[]column; annotating the callback param asanydiscards that inference instead of typing it asstring.♻️ Suggested fix
- if (!emailDomain || !classroom.allowedEmailDomains.some((d: any) => emailDomain === d.toLowerCase())) { + if (!emailDomain || !classroom.allowedEmailDomains.some((d: string) => emailDomain === d.toLowerCase())) {🤖 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/rooms/join/route.ts` at line 47, The callback in the allowed email domain check is typed with any, which drops the inferred string type from allowedEmailDomains. Update the some() callback in the join route to use an explicit string type for the domain element, and keep the lowercase comparison logic intact so the check remains type-safe in the join handler.src/app/api/rooms/members/route.ts (1)
91-95: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
anycasts discard the inferred select shape.
membersis a well-typed Drizzle result (id,userEmail,role,joinedAt); casting the mapped element toanyloses that safety without a corresponding need (destructuring{ id, ...m }works fine with the inferred type).🤖 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/rooms/members/route.ts` around lines 91 - 95, The `any` casts in the `members.map` branches are unnecessary and discard the inferred Drizzle select shape. Remove the `any` annotations from the mapped element in the members mapping logic and rely on the existing inferred types from the `members` query; keep the `{ id, ...m }` destructuring and the `isOwner` mapping in place so the result stays type-safe.src/app/api/teacher/analytics/route.ts (1)
32-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
anytyping widens previously-inferred DB result types.
teacherMembershipIds/selectedClassroomIdsderive from typed Drizzle selects; typing the callback params asany(rather than the actual row shape, e.g.{ classroomId: number; role: string }/{ id: number; ... }) loses compile-time safety. If this was needed to satisfylet classroomsList = []'s implicit-any inference, annotating that declaration (e.g.let classroomsList: { id: number; name: string; university: string }[] = [];) would be a more targeted fix than casting each callback toany.Also applies to: 95-95
🤖 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/teacher/analytics/route.ts` around lines 32 - 33, The `any` annotations in the `teacherMembershipIds` and `selectedClassroomIds` filter/map callbacks are widening the typed Drizzle results and removing compile-time safety. Replace those callback parameter types with the actual row shapes inferred from the selects, and fix the root cause by typing `classroomsList` explicitly in the route so it no longer forces `any`. Keep the change localized to the `route.ts` data-processing code and preserve the existing `.filter(...)` / `.map(...)` flow.src/inngest/functions.ts (1)
233-233: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
any-typed.mapfor extractingid.Both digest functions cast
ptoanyjust to read.id, which is already typed on thependingselect result (id: pendingNotificationsTable.id). No need to widen the type here.Also applies to: 317-317
🤖 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/inngest/functions.ts` at line 233, The `pending.map((p: any) => p.id)` usage in the digest functions is unnecessarily widening the type, since the selected `pending` result already includes `id`; remove the `any` cast and use the existing inferred item type so `id` is read directly. Update both occurrences in the digest flow (the `notificationIds` extraction in the related functions, including the second matching `pending.map` site) and keep the mapping logic the same while preserving type safety.src/app/api/teacher/insights/route.ts (1)
102-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIntermediate row typing loosened to
any.
row/r/tcome from typed Drizzle selects ({ topic, subject, unresolvedCount }/{ id }/{ topic, subject, totalCount }). Casting toanyhere means a mismatch against the finalWeakTopic[]shape (e.g.topic: string | null) wouldn't be caught by the compiler.Also applies to: 117-123
🤖 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/teacher/insights/route.ts` at line 102, The intermediate row typing was loosened to any in the insights route, which hides type mismatches between the Drizzle select results and the final WeakTopic[] shape. Update the mapping callbacks in the teacher insights handler (including the unresolvedPerTopic and related topic rows) to use the inferred select result types instead of any, so fields like topic, subject, and count remain compiler-checked. Keep the transformations in the same functions/mappings, but preserve the typed row shape all the way through to the WeakTopic construction.src/__tests__/inngest/digest-functions.test.ts (1)
119-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffUse the supported Inngest test API instead of
.fnsendDailyDigest.fn({ step })reaches into an internal property, so these tests can break on aninngestupgrade. Switch to@inngest/test(InngestTestEngine/t.execute()) or export the handler separately. Same applies to the other.fncall below.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/inngest/digest-functions.test.ts` at line 119, The digest tests are calling the internal `.fn` property on `sendDailyDigest`, which is not supported and can break on Inngest upgrades. Update these tests to use the supported `@inngest/test` API via `InngestTestEngine` and `t.execute()`, or refactor to export the handler separately and invoke that instead. Apply the same change to both `sendDailyDigest.fn` usages in `digest-functions.test`.
🤖 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.
Nitpick comments:
In `@src/__tests__/inngest/digest-functions.test.ts`:
- Line 119: The digest tests are calling the internal `.fn` property on
`sendDailyDigest`, which is not supported and can break on Inngest upgrades.
Update these tests to use the supported `@inngest/test` API via
`InngestTestEngine` and `t.execute()`, or refactor to export the handler
separately and invoke that instead. Apply the same change to both
`sendDailyDigest.fn` usages in `digest-functions.test`.
In `@src/app/api/admin/moderation/route.ts`:
- Around line 40-43: The `flagsPerDay.map` callback is using `any`, which throws
away the inferred shape from the preceding `db.select({ date, count })` query.
Remove the `any` annotation on the callback parameter (or replace it with the
inferred item type such as `typeof flagsPerDay[number]`) so the
`formattedFlagsPerDay` mapping in `route.ts` keeps TypeScript’s type-checking
while preserving the existing date/count transformation.
In `@src/app/api/analytics/route.ts`:
- Line 58: Remove the unnecessary `any` annotations in `route.ts` callbacks so
the typed query results from `db.select(...)` keep their inferred field types.
Update the array helpers in `orgClassrooms.map`, `userMemberships.map`,
`mostAskedTopics.map`, `weakTopics.filter`, and `topContributors.map` to use the
inferred element types from those variables instead of `any`, keeping the same
logic but allowing TypeScript to validate property access like `id`,
`classroomId`, and `topic`.
In `@src/app/api/doubts/route.ts`:
- Line 128: Remove the unnecessary any annotations from the map/reduce callbacks
in the doubts route and rely on the inferred Drizzle result types instead.
Update the callbacks around userBookmarks, userLikes, tagRows, and doubts to use
the existing typed objects directly so fields like doubtId, userId, and tag data
remain compile-time checked. Keep the logic unchanged in the route handlers and
helper expressions, just restore proper typing in the affected callbacks and
reducers.
- Around line 139-145: The pagination logic in route.ts still allows an
unbounded OFFSET because `offset` is derived directly from `offsetStr` or
`pageStr` without any maximum, so large queries can still hit Postgres/Neon
timeouts even with `limit` capped. Update the pagination flow around
`parsePositiveInt`, `offset`, and `page` to either enforce a maximum offset or
switch the `doubtsTable` query to cursor/keyset pagination, keeping the existing
`limit` cap as-is and ensuring the query cannot request arbitrarily large skips.
In `@src/app/api/rooms/join/route.ts`:
- Line 47: The callback in the allowed email domain check is typed with any,
which drops the inferred string type from allowedEmailDomains. Update the some()
callback in the join route to use an explicit string type for the domain
element, and keep the lowercase comparison logic intact so the check remains
type-safe in the join handler.
In `@src/app/api/rooms/members/route.ts`:
- Around line 91-95: The `any` casts in the `members.map` branches are
unnecessary and discard the inferred Drizzle select shape. Remove the `any`
annotations from the mapped element in the members mapping logic and rely on the
existing inferred types from the `members` query; keep the `{ id, ...m }`
destructuring and the `isOwner` mapping in place so the result stays type-safe.
In `@src/app/api/teacher/analytics/route.ts`:
- Around line 32-33: The `any` annotations in the `teacherMembershipIds` and
`selectedClassroomIds` filter/map callbacks are widening the typed Drizzle
results and removing compile-time safety. Replace those callback parameter types
with the actual row shapes inferred from the selects, and fix the root cause by
typing `classroomsList` explicitly in the route so it no longer forces `any`.
Keep the change localized to the `route.ts` data-processing code and preserve
the existing `.filter(...)` / `.map(...)` flow.
In `@src/app/api/teacher/insights/route.ts`:
- Line 102: The intermediate row typing was loosened to any in the insights
route, which hides type mismatches between the Drizzle select results and the
final WeakTopic[] shape. Update the mapping callbacks in the teacher insights
handler (including the unresolvedPerTopic and related topic rows) to use the
inferred select result types instead of any, so fields like topic, subject, and
count remain compiler-checked. Keep the transformations in the same
functions/mappings, but preserve the typed row shape all the way through to the
WeakTopic construction.
In `@src/configs/schema.ts`:
- Line 48: The Membership `role` field in the schema is only validated in
application code, so add a DB-level constraint in the schema definition to
restrict it to the allowed roles. Update the `role` column in
`src/configs/schema.ts` (and any related migration/schema setup) to use either a
Postgres enum or a check constraint so inserts through any path cannot store
invalid values. Keep the existing default of member and ensure the constraint
matches the role values used by the analytics route and other membership logic.
- Around line 34-63: Update the schema in organizationsTable and
organizationMembershipsTable to use proper user references: change
organizationMembershipsTable’s user FK to point at usersTable.id through a
userId-style column if needed, rather than usersTable.email, and add a foreign
key for organizationsTable.ownerEmail to the users table so the owner is
constrained too. Keep the existing table definitions and unique constraints
intact, and adjust any related column names or references in the schema
accordingly.
In `@src/inngest/functions.ts`:
- Line 233: The `pending.map((p: any) => p.id)` usage in the digest functions is
unnecessarily widening the type, since the selected `pending` result already
includes `id`; remove the `any` cast and use the existing inferred item type so
`id` is read directly. Update both occurrences in the digest flow (the
`notificationIds` extraction in the related functions, including the second
matching `pending.map` site) and keep the mapping logic the same while
preserving type safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b7279b17-0440-453c-bfcc-4172ba0d6103
📒 Files selected for processing (38)
drizzle/0017_audit_logs.sqldrizzle/0018_silky_gateway.sqldrizzle/meta/_journal.jsonsrc/__tests__/api/teacher-insights.test.tssrc/__tests__/configs/db.test.tssrc/__tests__/inngest/digest-functions.test.tssrc/app/api/admin/moderation/route.tssrc/app/api/admin/overview/route.tssrc/app/api/analytics/export/route.tssrc/app/api/analytics/personal/route.tssrc/app/api/analytics/route.tssrc/app/api/bookmarks/route.tssrc/app/api/classrooms/[id]/export/route.tssrc/app/api/doubts/[id]/upvote/route.tssrc/app/api/doubts/action/[id]/route.tssrc/app/api/doubts/check-similarity/route.tssrc/app/api/doubts/route.tssrc/app/api/karma/route.tssrc/app/api/organizations/route.tssrc/app/api/profile/route.tssrc/app/api/recommendations/route.tssrc/app/api/replies/vote/route.tssrc/app/api/resume-analyzer/history/route.tssrc/app/api/roadmap/history/route.tssrc/app/api/rooms/join/route.tssrc/app/api/rooms/members/route.tssrc/app/api/rooms/route.tssrc/app/api/teacher/analytics/route.tssrc/app/api/teacher/insights/route.tssrc/configs/db.tsxsrc/configs/schema.tssrc/inngest/functions.tssrc/inngest/karma.tssrc/lib/ai/embeddings.tssrc/lib/karma-utils.tssrc/lib/moderation.tssrc/lib/notifications/service.tssrc/lib/utils.ts
cae03f3 to
0f1d2be
Compare
|
CodeAnt AI Incremental review completed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@jest.setup.ts`:
- Line 89: The Jest mock for "`@/lib/ratelimit/ratelimit`" is missing the
aiDailyLimiter export, so imports from that module can resolve to undefined.
Update the mock in jest.setup.ts to include aiDailyLimiter alongside the other
mocked limiters, using the same mocking pattern as the existing exports so tests
depending on it receive a valid mocked value.
🪄 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: 66e71fbb-73ef-4f0b-a923-76f8e71c5f2e
📒 Files selected for processing (9)
drizzle/0008_audit_logs.sqldrizzle/0013_silky_gateway.sqljest.setup.tssrc/__tests__/api/teacher-insights.test.tssrc/__tests__/configs/db.test.tssrc/__tests__/inngest/digest-functions.test.tssrc/app/api/doubts/route.tssrc/app/api/invites/[token]/join/route.tssrc/lib/utils/utils.ts
💤 Files with no reviewable changes (2)
- drizzle/0008_audit_logs.sql
- drizzle/0013_silky_gateway.sql
✅ Files skipped from review due to trivial changes (3)
- src/app/api/invites/[token]/join/route.ts
- src/tests/api/teacher-insights.test.ts
- src/tests/configs/db.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/api/doubts/route.ts
- src/tests/inngest/digest-functions.test.ts
| })); | ||
|
|
||
| jest.mock("@/lib/ratelimit", () => { | ||
| jest.mock("@/lib/ratelimit/ratelimit", () => { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the full mock factory return object in jest.setup.ts
sed -n '89,120p' jest.setup.tsRepository: knoxiboy/DoubtDesk
Length of output: 799
Add aiDailyLimiter to the Jest mock. The mock currently omits this export, so anything importing it from @/lib/ratelimit/ratelimit will receive undefined.
🤖 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 `@jest.setup.ts` at line 89, The Jest mock for "`@/lib/ratelimit/ratelimit`" is
missing the aiDailyLimiter export, so imports from that module can resolve to
undefined. Update the mock in jest.setup.ts to include aiDailyLimiter alongside
the other mocked limiters, using the same mocking pattern as the existing
exports so tests depending on it receive a valid mocked value.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -p 'ratelimit' src/lib/ratelimit
cat src/lib/ratelimit/ratelimit.ts 2>/dev/null
rg -n "aiDailyLimiter" -g '*.ts' -g '*.tsx'Length of output: 5107
@knoxiboy, confirmed — aiDailyLimiter is exported from src/lib/ratelimit/ratelimit.ts but missing from the Jest mock. Here's the update for jest.setup.ts:
jest.mock("`@/lib/ratelimit/ratelimit`", () => {
const createLimiter = () => ({
limit: jest.fn().mockResolvedValue({
success: true,
limit: 100,
remaining: 99,
reset: Date.now() + 60_000,
}),
});
return {
aiLimiter: createLimiter(),
aiDailyLimiter: createLimiter(),
generalLimiter: createLimiter(),
emailNotificationLimiter: createLimiter(),
videoLimiter: createLimiter(),
redisClient: {
setnx: jest.fn().mockResolvedValue(1),
del: jest.fn().mockResolvedValue(1),
expire: jest.fn().mockResolvedValue(1),
},
};
});This adds the aiDailyLimiter mock alongside the existing limiters so any code importing it from @/lib/ratelimit/ratelimit receives a valid mocked value instead of undefined.
| })); | ||
|
|
||
| jest.mock("@/lib/ratelimit", () => { | ||
| jest.mock("@/lib/ratelimit/ratelimit", () => { |
|
@anshul23102 Resolve conflicts |
github-actions
Bot
added
merge-conflict
- Both branches implemented equivalent tag-grouping logic for the batched tag lookup; kept upstream's version, which types the reduce accumulator directly instead of using inline 'as' casts - Core fix from this PR (clamping the query limit via parsePositiveInt(limitStr, 20, 100)) is unaffected and intact
|
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 · |
github-actions
Bot
added
size/xs
and removed
merge-conflict
|
CodeAnt AI Incremental review completed. |
done |
- Same tag-grouping reduce block conflict as PR knoxiboy#780: kept upstream's cleaner typed version, both were functionally equivalent - Core fix in this PR (atomic 3-strike moderation via handleModerationViolation) is unaffected and merged in cleanly
github-actions
Bot
added
gssoc:approved
knoxiboy
added
level:beginner
|
Hey @knoxiboy! 👋 Ready for review on #780: Enforce maximum query limit on public doubt board (issue #755). This PR fixes a real bug in Suggested GSSoC labels (to help with contribution scoring):
This one includes both a bug fix and feature hardening! 🎯 |
- Add optional max parameter to parsePositiveInt for limit enforcement - Clamp the doubt board's limit query param to 100 to prevent Neon serverless timeouts on popular subjects (issue knoxiboy#755) - Rebuilt on current upstream/main; main was restructured twice while this branch was open (route groups, lib/ moved into subdirectories), which is why earlier pushes kept showing as conflicting - Fixed jest.setup.ts: the restructure moved lib/ratelimit.ts into lib/ratelimit/ratelimit.ts but left the jest.mock() path pointing at the old bare path, which broke all 38 test suites on main itself (confirmed on a clean checkout of upstream/main, unrelated to this branch's own change) - Removed orphan duplicate migration files not tracked in journal.json - Fixed pre-existing TypeScript errors (untyped reduce/generic calls, readonly process.env delete) - Fixed pre-existing test expectations in teacher-insights.test.ts and inngest digest-functions.test.ts to match actual endpoint behavior - Verified: tsc clean, 193/193 tests pass, production build succeeds Signed-off-by: Anshul Jain <anshul23102@iiitd.ac.in>
2 participants
User description
Prevents resource exhaustion attacks on public doubt board.
Closes #755
Summary by CodeRabbit
limitvalue for the doubts API at 100 to prevent overly large requests and keep results consistent.CodeAnt-AI Description
Cap public doubt board results to prevent oversized requests
What Changed
Impact
✅ Fewer oversized doubt board requests✅ Lower risk of slow public board loads✅ More reliable load handling on popular subjects💡 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.