Conversation
- Add monthlyLimit to RateLimitConfig (free: 3000, tier_1: 7000, tier_2: 20000) - Update getUsageStats backend query to aggregate monthly usage - Add Monthly Usage card to dashboard and API Keys pages - Update settings page to show monthly limit info - Enhance useBreakpoint hook with 3xl (1920px) and 4xl (2560px) breakpoints - Add isWideScreen, isUltraWide, and screenCategory helpers - Add CSS auto-fit grid utilities for automatic responsive layouts - Support 2K and 4K resolution screens with proper spacing - Update API docs with complete tier rate limits table
Rate Limits by Tier: - Free: 65/min, 3,300/day, 95,000/month, burst +15 (max 80) - Tier 1: 130/min, 6,600/day, 195,000/month, burst +30 (max 160) - Tier 2: 145/min, 7,500/day, 215,000/month, burst +45 (max 190) VPS Safety Caps: - Max 20 requests/second (global) - Max 5 concurrent requests per API key Changes: - Expand RateLimitConfig with minuteLimit, dailyLimit, monthlyLimit - Add burst config: burstBonus, burstWindowSeconds, maxBurstTotal - Add VpsSafetyCaps interface and VPS_SAFETY_CAPS constant - Rewrite rate-limit middleware with multi-level checking - Add second-level tracking for VPS safety cap - Update all dashboard components to use new limit fields - Update API documentation with comprehensive rate limit info
- Drop unique index on email field (via cleanup script) - Update User type to allow email: string | null - Change context.ts to store null instead of empty string - Prevents E11000 duplicate key errors for social login users without email
…acker-for-1-month Kairul/kal 38 feat add limit tracker for 1 month
📝 WalkthroughWalkthroughThe PR introduces a multi-level rate-limiting system with per-minute, daily, and monthly quotas alongside burst bonuses and VPS safety caps. It updates backend middleware to track granular time windows, extends shared type definitions, adds monthly usage tracking and responsive grid utilities to frontend dashboards, and adjusts user data models to support nullable email fields. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/kal-backend/src/routers/api-keys.ts (1)
168-187: Monthly aggregation logic is correct; consider index optimization for scale.The implementation correctly aggregates daily counts for the current month. However, the regex match on
datefield ($regex: ^${currentMonth}) may not utilize indexes efficiently.For better query performance at scale, consider a range-based match:
🔎 Optional optimization using date range
const monthlyAggregation = await ctx.db .collection<RateLimitUsage>("rate_limit_usage") .aggregate([ { $match: { userId: ctx.userId, - date: { $regex: `^${currentMonth}` }, + date: { + $gte: `${currentMonth}-01`, + $lte: `${currentMonth}-31` + }, }, }, ... ])This allows MongoDB to use a compound index on
(userId, date)more effectively.packages/kal-frontend/src/app/dashboard/settings/client.tsx (1)
109-121: Consider responsive breakpoints for the 3-column grid.The fixed
grid-cols-3may cause cramped layouts on smaller screens. Consider using responsive classes for better mobile experience.🔎 Suggested responsive fix
-<div className="grid grid-cols-3 gap-4 pt-4 border-t border-dark-border"> +<div className="grid grid-cols-1 sm:grid-cols-3 gap-4 pt-4 border-t border-dark-border">packages/kal-shared/src/types/index.ts (1)
25-28: Burst configuration fields are defined but not enforced.The
burstBonus,burstWindowSeconds, andmaxBurstTotalfields are defined here and values are set inRATE_LIMITS, but the rate-limit middleware (packages/kal-backend/src/middleware/rate-limit.ts) doesn't implement any burst logic. TheRateLimitUsageinterface also hasburstWindowStartandburstCountfields that aren't being used.If burst limiting is planned for future implementation, consider adding a TODO comment. Otherwise, remove these unused fields to avoid confusion.
packages/kal-backend/src/middleware/rate-limit.ts (4)
140-150: Consider addingmonthlyLimitfor consistency.The "second" limit response doesn't include
monthlyLimit, but the "daily" limit response does. For consistent header generation, consider adding it.🔎 Proposed fix
return { limited: true, retryAfter: 1, // Wait 1 second limitType: "second", secondCount, minuteCount: usage.minuteCount, dailyCount: usage.dailyCount, minuteLimit: limits.minuteLimit, dailyLimit: limits.dailyLimit, + monthlyLimit: limits.monthlyLimit, };
162-171: MissingmonthlyLimitin minute-limited response.Same as the second-limit response,
monthlyLimitis missing here but included in the daily-limit response.🔎 Proposed fix
return { limited: true, retryAfter, limitType: "minute", minuteCount: usage.minuteCount, dailyCount: usage.dailyCount, minuteLimit: limits.minuteLimit, dailyLimit: limits.dailyLimit, + monthlyLimit: limits.monthlyLimit, };
182-191: MissingminuteLimitin daily-limited response.This response includes
monthlyLimitbut omitsminuteLimit, creating inconsistency with other responses.🔎 Proposed fix
return { limited: true, retryAfter: secondsUntilMidnight, limitType: "daily", dailyCount: usage.dailyCount, minuteCount: usage.minuteCount, + minuteLimit: limits.minuteLimit, dailyLimit: limits.dailyLimit, monthlyLimit: limits.monthlyLimit, };
225-228: Uselogger.errorfor consistency.Line 227 uses
console.errorwhile the rest of the file useslogger. For consistent logging and structured output, consider using the logger.🔎 Proposed fix
// Log stack trace for debugging if (errorStack) { - console.error("Rate limit error stack trace:", errorStack); + logger.error("Rate limit error stack trace", { stack: errorStack }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/api.mdpackages/kal-backend/src/lib/context.tspackages/kal-backend/src/middleware/rate-limit.tspackages/kal-backend/src/routers/api-keys.tspackages/kal-frontend/src/app/dashboard/api-keys/client.tsxpackages/kal-frontend/src/app/dashboard/client.tsxpackages/kal-frontend/src/app/dashboard/settings/client.tsxpackages/kal-frontend/src/app/globals.csspackages/kal-frontend/src/hooks/useBreakpoint.tspackages/kal-shared/src/types/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/kal-backend/src/routers/api-keys.ts (1)
packages/kal-shared/src/types/index.ts (1)
RateLimitUsage(69-90)
packages/kal-backend/src/middleware/rate-limit.ts (1)
packages/kal-shared/src/types/index.ts (2)
RateLimitUsage(69-90)VPS_SAFETY_CAPS(64-67)
🪛 markdownlint-cli2 (0.18.1)
docs/api.md
330-330: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (18)
packages/kal-frontend/src/hooks/useBreakpoint.ts (1)
5-6: LGTM! Well-structured breakpoint extension.The implementation correctly extends the breakpoint system with 3xl (1920px) and 4xl (2560px) thresholds for wide/ultrawide screens. The SSR-safe defaults are preserved, and the
getScreenCategoryfunction provides a clean abstraction for broader screen classifications.Also applies to: 14-15, 59-61, 81-82, 91-97, 106-108
packages/kal-backend/src/lib/context.ts (1)
34-34: LGTM! Correct handling of nullable email.Using
nullinstead of an empty string for missing emails is the correct approach. This prevents duplicate key errors on unique email indexes for social login users without email addresses and aligns with theUsertype definition (email: string | null).docs/api.md (1)
305-324: LGTM! Comprehensive rate limit documentation.The updated documentation clearly explains the multi-level rate limiting system with tier-specific limits, burst configuration, and VPS safety caps. This provides excellent guidance for API consumers.
packages/kal-frontend/src/app/globals.css (2)
884-935: LGTM! Well-designed responsive grid utilities.The auto-fit grid system using CSS custom properties is a clean, maintainable approach. The
minmax()pattern with CSS variables allows easy customization while providing automatic column adjustment based on available space.
958-1017: LGTM! Progressive enhancement for containers and ultra-wide screens.The fluid container utilities with progressive padding and the ultra-wide
stats-gridadjustments provide appropriate enhancements for larger displays without breaking smaller viewports.packages/kal-frontend/src/app/dashboard/api-keys/client.tsx (2)
114-116: LGTM! Consistent monthly usage implementation.The monthly usage metrics follow the same pattern as daily usage, maintaining code consistency. The progress bar implementation and number formatting with
toLocaleString()provide a good user experience.Also applies to: 155-167
128-139: LGTM! Good adoption of responsive grid utilities.Using
grid-auto-fit-mdfrom the new CSS utilities provides automatic responsive behavior without manual breakpoint management. The tier info card now comprehensively displays all rate limit tiers (minute/daily/monthly).packages/kal-frontend/src/app/dashboard/client.tsx (2)
53-55: LGTM! Consistent monthly usage implementation across dashboard.The monthly usage metrics and UI card implementation are consistent with the API keys page, providing a unified experience. The calculations follow the same pattern as daily usage metrics.
Also applies to: 97-109
70-70: LGTM! Good use of responsive grid utilities.The adoption of
grid-auto-fit-mdfor stats cards andgrid-auto-fit-lgfor quick actions provides appropriate responsive behavior with automatically adjusted column counts based on available space.Also applies to: 125-125
packages/kal-shared/src/types/index.ts (3)
6-14: LGTM!The
email: string | nullchange properly supports social login users who may not have an email address. This aligns with the PR objective to handle nullable emails and prevent duplicate-key errors.
31-56: LGTM!The rate limit configurations are well-structured and internally consistent. The
maxBurstTotalvalues correctly equalminuteLimit + burstBonusfor each tier.
69-90: LGTM!The
RateLimitUsageinterface properly defines required fields for daily/minute tracking and optional fields for second-level tracking and burst windows, supporting backward compatibility with existing documents.packages/kal-backend/src/middleware/rate-limit.ts (6)
7-21: LGTM!The
RateLimitResultinterface is well-structured with clear separation of counts and limits. The optional fields allow flexible response composition based on which limit was triggered.
36-56: LGTM!Time window calculations are correct.
minuteStart.setSeconds(0, 0)properly resets both seconds and milliseconds, whilesecondStart.setMilliseconds(0)resets only milliseconds for second-level granularity.
106-129: LGTM!The upsert pattern is correct for MongoDB driver 6.x which returns the document directly. The null check provides a safe fallback, though as noted this shouldn't occur with
upsert: true.
193-204: LGTM!The comment clearly documents that monthly limits are checked via aggregation elsewhere, and the success response includes all relevant counts and limits for header generation.
247-252: LGTM!The
getSecondsUntilMidnightimplementation correctly usessetUTCHours(24, 0, 0, 0)to roll to the next day's midnight UTC.
257-286: LGTM!Header generation is well-structured. The absence of
X-RateLimit-Remaining-Monthlyis consistent with the design decision to calculate monthly usage via aggregation elsewhere.
| ``` | ||
| X-RateLimit-Limit: 100 | ||
| X-RateLimit-Remaining: 95 | ||
| X-RateLimit-Reset: 1703548800 | ||
| X-RateLimit-Limit-Minute: 65 | ||
| X-RateLimit-Remaining-Minute: 60 | ||
| X-RateLimit-Limit-Daily: 3300 | ||
| X-RateLimit-Remaining-Daily: 3295 | ||
| X-RateLimit-Limit-Monthly: 95000 | ||
| Retry-After: 5 | ||
| X-RateLimit-Type: minute | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The static analysis tool correctly identifies that the fenced code block lacks a language specifier. Adding one improves syntax highlighting and accessibility.
🔎 Proposed fix
-```
+```http
X-RateLimit-Limit-Minute: 65
X-RateLimit-Remaining-Minute: 60🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
330-330: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/api.md around lines 330 to 338 the fenced code block is missing a
language specifier; update the opening fence to include the language (e.g.,
change ``` to ```http) so the block becomes a fenced "http" code block for
proper syntax highlighting and accessibility.
| * Rate limit hierarchy (checked in order): | ||
| * 1. VPS Safety Cap: 20 requests/second (global) | ||
| * 2. Minute limit: varies by tier (65/130/110 per minute) | ||
| * 3. Daily limit: varies by tier | ||
| * 4. Monthly limit: varies by tier |
There was a problem hiding this comment.
Fix outdated comment: tier_2 minute limit is 145, not 110.
The comment states 65/130/110 per minute, but according to RATE_LIMITS in the shared types, tier_2 has minuteLimit: 145.
🔎 Proposed fix
* Rate limit hierarchy (checked in order):
* 1. VPS Safety Cap: 20 requests/second (global)
-* 2. Minute limit: varies by tier (65/130/110 per minute)
+* 2. Minute limit: varies by tier (65/130/145 per minute)
* 3. Daily limit: varies by tier
* 4. Monthly limit: varies by tier📝 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.
| * Rate limit hierarchy (checked in order): | |
| * 1. VPS Safety Cap: 20 requests/second (global) | |
| * 2. Minute limit: varies by tier (65/130/110 per minute) | |
| * 3. Daily limit: varies by tier | |
| * 4. Monthly limit: varies by tier | |
| * Rate limit hierarchy (checked in order): | |
| * 1. VPS Safety Cap: 20 requests/second (global) | |
| * 2. Minute limit: varies by tier (65/130/145 per minute) | |
| * 3. Daily limit: varies by tier | |
| * 4. Monthly limit: varies by tier |
🤖 Prompt for AI Agents
In packages/kal-backend/src/middleware/rate-limit.ts around lines 26 to 30, the
inline comment listing per-minute tier limits is outdated (shows "65/130/110 per
minute") — update that comment to reflect the current RATE_LIMITS which set
tier_2 minuteLimit to 145 (e.g., "65/130/145 per minute") so the documentation
matches the shared types; edit only the comment block to replace 110 with 145
and keep wording consistent.
| try { | ||
| // First, get current usage to check minute window | ||
| // First, get current usage to check windows | ||
| const existingUsage = await collection.findOne({ _id: compositeId as unknown as string }); | ||
|
|
||
| // Check if we're in a new minute window | ||
| // Check if we're in new time windows | ||
| const isNewMinute = | ||
| !existingUsage?.minuteWindow || | ||
| new Date(existingUsage.minuteWindow).getTime() < minuteStart.getTime(); | ||
|
|
||
| const isNewSecond = | ||
| !existingUsage?.secondWindow || | ||
| new Date(existingUsage.secondWindow).getTime() < secondStart.getTime(); | ||
|
|
||
| // Use atomic upsert - single operation for both create and update | ||
| // This prevents race conditions and duplicate key errors | ||
| const updateDoc = isNewMinute || !existingUsage | ||
| ? { | ||
| // New minute or first request: reset minute counter | ||
| $inc: { dailyCount: 1 }, | ||
| $set: { | ||
| minuteWindow: minuteStart, | ||
| minuteCount: 1, | ||
| updatedAt: now, | ||
| }, | ||
| $setOnInsert: { | ||
| userId, | ||
| date: today, | ||
| }, | ||
| } | ||
| : { | ||
| // Same minute: increment both counters | ||
| $inc: { dailyCount: 1, minuteCount: 1 }, | ||
| $set: { updatedAt: now }, | ||
| }; | ||
| // Build update document based on window states | ||
| interface UpdateOperation { | ||
| $inc: Record<string, number>; | ||
| $set: Record<string, Date | number>; | ||
| $setOnInsert?: Record<string, string>; | ||
| } | ||
|
|
||
| const updateDoc: UpdateOperation = { | ||
| $inc: { dailyCount: 1 }, | ||
| $set: { updatedAt: now }, | ||
| }; | ||
|
|
||
| // Handle minute window | ||
| if (isNewMinute || !existingUsage) { | ||
| updateDoc.$set.minuteWindow = minuteStart; | ||
| updateDoc.$set.minuteCount = 1; | ||
| } else { | ||
| updateDoc.$inc.minuteCount = 1; | ||
| } | ||
|
|
||
| // Handle second window (for VPS safety cap) | ||
| if (isNewSecond || !existingUsage) { | ||
| updateDoc.$set.secondWindow = secondStart; | ||
| updateDoc.$set.secondCount = 1; | ||
| } else { | ||
| updateDoc.$inc.secondCount = 1; | ||
| } | ||
|
|
||
| // Set on insert for new documents | ||
| if (!existingUsage) { | ||
| updateDoc.$setOnInsert = { | ||
| userId, | ||
| date: today, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Race condition: concurrent requests may lose counts.
The pattern of findOne followed by findOneAndUpdate creates a race window. If two concurrent requests both see existingUsage as null (or both see the same stale minuteWindow), they'll both execute $set.minuteCount = 1 instead of incrementing, losing a count.
Consider using a single atomic update with conditional operators like $cond in an aggregation pipeline update, or accept the minor inaccuracy for simplicity. Given the VPS safety cap of 20 req/s, this race is likely rare but could affect burst scenarios.
🔎 Alternative: Use aggregation pipeline for atomic window reset
MongoDB 4.2+ supports aggregation pipelines in updates for conditional logic:
const result = await collection.findOneAndUpdate(
{ _id: compositeId },
[
{
$set: {
userId: { $ifNull: ["$userId", userId] },
date: { $ifNull: ["$date", today] },
updatedAt: now,
// Minute window: reset if new window, otherwise keep
minuteWindow: {
$cond: {
if: { $lt: [{ $ifNull: ["$minuteWindow", new Date(0)] }, minuteStart] },
then: minuteStart,
else: "$minuteWindow"
}
},
minuteCount: {
$cond: {
if: { $lt: [{ $ifNull: ["$minuteWindow", new Date(0)] }, minuteStart] },
then: 1,
else: { $add: [{ $ifNull: ["$minuteCount", 0] }, 1] }
}
},
// Similar for secondWindow/secondCount...
dailyCount: { $add: [{ $ifNull: ["$dailyCount", 0] }, 1] }
}
}
],
{ upsert: true, returnDocument: "after" }
);This ensures atomicity without a preliminary read.
🤖 Prompt for AI Agents
In packages/kal-backend/src/middleware/rate-limit.ts around lines 57 to 104, the
current findOne then findOneAndUpdate flow creates a race where concurrent
requests can both treat a window as "new" and reset counts (losing increments);
replace the two-step read+update with a single atomic findOneAndUpdate using an
aggregation-pipeline update that uses $set/$cond/$ifNull/$add to: set
userId/date on insert, set updatedAt, reset minuteWindow/secondWindow and set
minuteCount/secondCount to 1 when the stored window is older than the computed
window, otherwise add 1 to the existing counts, and always $add to dailyCount;
ensure options include upsert: true and returnDocument: "after" so the operation
is atomic and returns the updated counters.
| export const VPS_SAFETY_CAPS: VpsSafetyCaps = { | ||
| maxRequestsPerSecond: 20, | ||
| maxConcurrentPerApiKey: 5, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "maxConcurrentPerApiKey" --type tsRepository: Zen0space/Kal-Monorepo
Length of output: 212
🏁 Script executed:
rg -n "VPS_SAFETY_CAPS" --type tsRepository: Zen0space/Kal-Monorepo
Length of output: 489
🏁 Script executed:
rg -n "maxConcurrent" --type tsRepository: Zen0space/Kal-Monorepo
Length of output: 212
🏁 Script executed:
fd -name "*middleware*" -o -name "*rate*" --type fRepository: Zen0space/Kal-Monorepo
Length of output: 296
🏁 Script executed:
find . -name "*middleware*" -type f 2>/dev/nullRepository: Zen0space/Kal-Monorepo
Length of output: 125
🏁 Script executed:
rg -n "concurrent" -i --type tsRepository: Zen0space/Kal-Monorepo
Length of output: 212
🏁 Script executed:
rg -n "api.?key.*middleware\|api.?key.*rate" -i --type tsRepository: Zen0space/Kal-Monorepo
Length of output: 48
🏁 Script executed:
cat -n ./packages/kal-backend/src/middleware/api-key-middleware.tsRepository: Zen0space/Kal-Monorepo
Length of output: 6356
Remove unused maxConcurrentPerApiKey or implement concurrent request limiting.
maxConcurrentPerApiKey: 5 is defined in VPS_SAFETY_CAPS but never enforced anywhere in the codebase. It is not used in rate-limit.ts or api-key-middleware.ts. Either implement the concurrent request limiting logic using this constant or remove it from the type definition and export to avoid misleading developers about the actual safety measures in place.
🤖 Prompt for AI Agents
packages/kal-shared/src/types/index.ts lines 64-67: VPS_SAFETY_CAPS currently
defines maxConcurrentPerApiKey:5 but this limit is never enforced; either remove
the property from the exported constant and the VpsSafetyCaps type, or implement
enforcement in the request pipeline: add a per-api-key in-flight
counter/semaphore (e.g., Map<apiKey, number> or a lightweight semaphore) that
increments at request start and decrements on finish/error, check against
VPS_SAFETY_CAPS.maxConcurrentPerApiKey and immediately respond with 429 when
exceeded, and ensure the counter is decremented in all code paths (including
error and timeouts) to avoid leaks.
1 participant
📝 Description
Brief description of what this PR does.
🔗 Related Issue
Fixes #(issue number)
🏷️ Type of Change
✅ Checklist
dev(notmain)pnpm lint:fixpnpm typecheck📸 Screenshots (if applicable)
Add screenshots to help explain your changes.
🧪 How to Test
Steps to test this PR:
📝 Additional Notes
Any additional information reviewers should know.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.