feat(ai): add cost-control kill switch and daily quota#585
Conversation
|
CodeAnt AI is reviewing your PR. |
|
@eshaanag 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. |
|
Warning Review limit reached
More reviews will be available in 9 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds an environment-driven AI kill-switch, a daily per-user aiDailyLimiter, provider error classification and standardized responses, integrates these checks into ask-ai, check-similarity, and ai-career-chat-agent routes, expands tests, and adds a Groq dummy-key fallback for production imports. ChangesAI Kill-Switch & Cost Control
Sequence DiagramsequenceDiagram
participant Client
participant Route as "API Route (ask-ai / career / similarity)"
participant enforce as "enforceAiAvailability"
participant aiDaily as "aiDailyLimiter"
participant Groq as "Groq (provider / axios.post)"
Client->>Route: POST request
Route->>Route: parse & validate request
Route->>enforce: enforceAiAvailability(identifier)
enforce->>aiDaily: aiDailyLimiter.limit(ai-daily:<sha256>)
alt limiter allows
aiDaily-->>enforce: success:true
enforce-->>Route: null (proceed)
Route->>Groq: axios.post(...)
Groq-->>Route: completion / error
alt provider error
Route->>enforce: buildAiProviderErrorResponse(error)
enforce-->>Route: 503/500 standardized response
end
else limiter denies or disabled
aiDaily-->>enforce: success:false / feature disabled
enforce-->>Route: 429 or 503 response
end
Route-->>Client: standardized response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai review |
|
Hosted CI update: the new |
|
CodeAnt AI finished reviewing your PR. |
|
Final CI classification for bf45a70: the new ai-career-chat-agent malformed-JSON regression test passes, as does the ai-kill-switch suite. Build, TypeScript, ESLint, security, dependency audit, and PR quality gate pass. The unit-test job has only the same five upstream baseline failures in rooms-members, doubts sorting/filtering, and DB config. The duplicate-invalid-handler workflow has zero jobs and fails identically on prior pushes, so it is repository automation rather than this patch. |
|
Hi @eshaanag! Thanks for your work on this. Before we can merge, please address the following:
Once these are fixed, the PR will be automatically evaluated again. Let us know if you need any help! |
|
CodeAnt AI is running Incremental review |
|
CodeAnt AI Incremental review completed. |
|
Reviewed the remaining feedback after the incremental review. The malformed-JSON provider-error issue is resolved in |
|
CodeAnt AI is running Incremental review |
|
CodeAnt AI Incremental review completed. |
This reverts commit e1933c9.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/__tests__/lib/ai-kill-switch.test.ts (1)
61-77: ⚡ Quick winAdd a test for limiter failure fallback (
AI_QUOTA_UNAVAILABLE).
enforceAiAvailabilityhas a catch branch for limiter failures, but this suite doesn’t assert it. A focused test here will lock the 503 fallback contract.Suggested test case
it("returns a retryable response when the daily quota is exhausted", async () => { @@ }); + + it("returns 503 when quota backend check fails", async () => { + mockDailyLimit.mockRejectedValueOnce(new Error("redis unavailable")); + + const response = await enforceAiAvailability("student@example.com"); + + expect(response?.status).toBe(503); + await expect(response?.json()).resolves.toEqual({ + error: "AI features are temporarily unavailable.", + code: "AI_QUOTA_UNAVAILABLE", + }); + });Also applies to: 101-101
🤖 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__/lib/ai-kill-switch.test.ts` around lines 61 - 77, Add a new unit test that exercises the limiter-failure catch path in enforceAiAvailability by making the mocked limiter (mockDailyLimit) simulate an internal failure (throw or resolve to a failure result) so the function returns the 503 fallback with code "AI_QUOTA_UNAVAILABLE"; specifically, call enforceAiAvailability("student@example.com") after configuring mockDailyLimit to fail, then assert response.status === 503, that response.headers.get("Retry-After") is truthy, and that response.json() resolves to { error: "AI quota unavailable. Please try again later.", code: "AI_QUOTA_UNAVAILABLE" } to lock the fallback contract.src/__tests__/configs/db.test.ts (2)
48-55: ⚡ Quick winConsider explicitly deleting
NEXT_PHASEfor clarity.Similar to the test at line 28-38, this test expects an error when
DATABASE_URLis missing andNEXT_PHASEis not'phase-production-build'. Consider addingdelete process.env.NEXT_PHASE;at line 51 for consistency and clarity.♻️ Proposed change
delete process.env.DATABASE_URL; process.env.NEXT_PUBLIC_NEON_DB_CONNECTION_STRING = 'postgresql://public-prefix/database'; + delete process.env.NEXT_PHASE; const { getDatabaseUrl } = require('`@/configs/database-url`');🤖 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__/configs/db.test.ts` around lines 48 - 55, The test should explicitly clear NEXT_PHASE before calling getDatabaseUrl to mirror the other test and make intent explicit: add a deletion of process.env.NEXT_PHASE (e.g., delete process.env.NEXT_PHASE) in the test that sets process.env.NEXT_PUBLIC_NEON_DB_CONNECTION_STRING and deletes process.env.DATABASE_URL so getDatabaseUrl() runs with NEXT_PHASE undefined and throws missingDatabaseUrlError; update the test around the getDatabaseUrl invocation to include this deletion for clarity and consistency.
28-38: ⚡ Quick winConsider explicitly deleting
NEXT_PHASEfor clarity.The test expects
getDatabaseUrl()to throw, which happens whenNEXT_PHASEis not'phase-production-build'. For clarity and consistency with the new test at line 57-64 that explicitly setsNEXT_PHASE, consider addingdelete process.env.NEXT_PHASE;at line 34 (before the require).♻️ Proposed change
} else { process.env.DATABASE_URL = databaseUrl; } + delete process.env.NEXT_PHASE; const { getDatabaseUrl } = require('`@/configs/database-url`');🤖 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__/configs/db.test.ts` around lines 28 - 38, The test for getDatabaseUrl should explicitly ensure NEXT_PHASE is unset so the code path that throws is deterministic; before requiring '`@/configs/database-url`' in the 'throws when DATABASE_URL is %p' it.each block, delete process.env.NEXT_PHASE to remove any inherited value (i.e., add delete process.env.NEXT_PHASE before the require call) so getDatabaseUrl() runs with NEXT_PHASE undefined and consistently throws missingDatabaseUrlError.src/__tests__/api/ai-career-chat-agent.test.ts (1)
29-44: ⚡ Quick winAdd a
null(and optionally array) body regression case.This test covers invalid JSON syntax, but not valid JSON with invalid shape. Adding
body: "null"helps lock the expected 400 behavior for the body-shape path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/api/ai-career-chat-agent.test.ts` around lines 29 - 44, The test only checks malformed JSON string but not valid JSON with wrong shape; add a regression case sending body: "null" (and optionally "[]") to the POST handler in the same test file so the handler returns 400 and the JSON error { error: "Invalid JSON body" } before calling enforceAiAvailability or axios.post; update or add an it(...) that constructs a Request to "/api/ai-career-chat-agent" with Content-Type "application/json" and body "null" (and another with "[]"), calls POST(request as any), asserts response.status === 400, response.json() equals the expected error, and that enforceAiAvailability and axios.post were not called to cover the body-shape path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/ai-career-chat-agent/route.tsx`:
- Around line 120-125: The current catch blocks log the full provider error
object ("AI Career Chat Provider Error:" and "AI Career Chat Error:"), which may
leak sensitive request metadata; update both handlers (the provider-specific
catch that calls buildAiProviderErrorResponse and the outer catch that returns
NextResponse.json) to log only a sanitized subset (e.g., error.code or
error.status and error.message) by extracting those fields from the caught error
(use type guards or optional chaining) and log a minimal object or formatted
string instead of the raw error; ensure buildAiProviderErrorResponse is passed
only sanitized details or that you construct the response using the sanitized
fields (status/code/message) rather than the original error object.
- Around line 25-35: The request JSON body may be null or a non-object (e.g.,
array) so accessing body.userInput can throw; after parsing via await
req.json(), validate that body is a plain object (not null and not an array)
before reading body.userInput and return NextResponse.json({ error: "userInput
is required" }, { status: 400 }) for invalid shapes. Update the handler around
the body/userInput logic (the variables body and userInput in route.tsx) to
check typeof body === "object" && body !== null && !Array.isArray(body) and only
then assign const userInput = body.userInput; otherwise return the 400 response.
- Around line 99-114: The axios.post call that sends messages to GROQ (the call
in route.tsx constructing model:"llama-3.3-70b-versatile" with messages built
from systemPrompt and userInput and Authorization via process.env.GROQ_API_KEY)
has no timeout; add a timeout value (e.g. timeout: 15000) to the request config
(the third argument to axios.post) so stalled upstreams fail fast; update the
axios.post options object to include timeout and ensure any existing headers
stay intact.
In `@src/app/api/doubts/check-similarity/route.ts`:
- Around line 38-41: The quota key currently uses the raw
x-forwarded-for/x-real-ip header in aiQuotaIdentifier (used by
enforceAiAvailability which builds keys like
ai-daily:${identifier.trim().toLowerCase()}), allowing clients to spoof/rotate
values to evade limits; change the logic in route.ts to obtain a trusted,
canonicalized client identifier: parse and validate the IP (e.g., use a
server-side trusted IP provided by your proxy or use a verified proxy-chain
parser and then canonicalize to a single IP string), fall back to a stable
server-generated identifier (e.g., "anonymous") if validation fails, and
preserve the existing behavior that overwrites aiQuotaIdentifier with the
authenticated user email when classroomId exists so enforceAiAvailability
receives only validated/canonical identifiers.
In `@src/lib/ai/kill-switch.ts`:
- Around line 71-73: The rate-limit key uses the raw identifier when calling
aiDailyLimiter.limit (`ai-daily:${identifier.trim().toLowerCase()}`); replace
that with a deterministic hash of the normalized identifier (e.g., trim +
toLowerCase then hash) before building the key so PII isn't stored in
Redis/analytics while quota behavior remains stable; update the call site where
`aiDailyLimiter.limit` is invoked and ensure the variable used in the key is the
hashed value (retain the same `ai-daily:` prefix).
---
Nitpick comments:
In `@src/__tests__/api/ai-career-chat-agent.test.ts`:
- Around line 29-44: The test only checks malformed JSON string but not valid
JSON with wrong shape; add a regression case sending body: "null" (and
optionally "[]") to the POST handler in the same test file so the handler
returns 400 and the JSON error { error: "Invalid JSON body" } before calling
enforceAiAvailability or axios.post; update or add an it(...) that constructs a
Request to "/api/ai-career-chat-agent" with Content-Type "application/json" and
body "null" (and another with "[]"), calls POST(request as any), asserts
response.status === 400, response.json() equals the expected error, and that
enforceAiAvailability and axios.post were not called to cover the body-shape
path.
In `@src/__tests__/configs/db.test.ts`:
- Around line 48-55: The test should explicitly clear NEXT_PHASE before calling
getDatabaseUrl to mirror the other test and make intent explicit: add a deletion
of process.env.NEXT_PHASE (e.g., delete process.env.NEXT_PHASE) in the test that
sets process.env.NEXT_PUBLIC_NEON_DB_CONNECTION_STRING and deletes
process.env.DATABASE_URL so getDatabaseUrl() runs with NEXT_PHASE undefined and
throws missingDatabaseUrlError; update the test around the getDatabaseUrl
invocation to include this deletion for clarity and consistency.
- Around line 28-38: The test for getDatabaseUrl should explicitly ensure
NEXT_PHASE is unset so the code path that throws is deterministic; before
requiring '`@/configs/database-url`' in the 'throws when DATABASE_URL is %p'
it.each block, delete process.env.NEXT_PHASE to remove any inherited value
(i.e., add delete process.env.NEXT_PHASE before the require call) so
getDatabaseUrl() runs with NEXT_PHASE undefined and consistently throws
missingDatabaseUrlError.
In `@src/__tests__/lib/ai-kill-switch.test.ts`:
- Around line 61-77: Add a new unit test that exercises the limiter-failure
catch path in enforceAiAvailability by making the mocked limiter
(mockDailyLimit) simulate an internal failure (throw or resolve to a failure
result) so the function returns the 503 fallback with code
"AI_QUOTA_UNAVAILABLE"; specifically, call
enforceAiAvailability("student@example.com") after configuring mockDailyLimit to
fail, then assert response.status === 503, that
response.headers.get("Retry-After") is truthy, and that response.json() resolves
to { error: "AI quota unavailable. Please try again later.", code:
"AI_QUOTA_UNAVAILABLE" } to lock the fallback contract.
🪄 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: 2df6b304-14a1-4fbe-ba7a-c1eee10244e9
📒 Files selected for processing (14)
.env.examplesrc/__tests__/api/ai-career-chat-agent.test.tssrc/__tests__/api/ask-ai.test.tssrc/__tests__/api/confusion-spike-detector.test.tssrc/__tests__/api/doubts-similarity.test.tssrc/__tests__/configs/db.test.tssrc/__tests__/lib/ai-kill-switch.test.tssrc/app/api/ai-career-chat-agent/route.tsxsrc/app/api/ask-ai/route.tssrc/app/api/doubts/check-similarity/route.tssrc/app/api/inngest/ConfusionSpikeDetector.tssrc/configs/database-url.tssrc/lib/ai/kill-switch.tssrc/lib/ratelimit.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/api/doubts/check-similarity/route.ts (1)
112-114:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSanitize provider error logging before writing to server logs.
Line 113 logs the raw provider error object. That can leak sensitive fields (e.g., request config/headers) into logs. Log only a safe subset (
message,status,code) and keep returningbuildAiProviderErrorResponse(err).Proposed fix
} catch (err) { - console.error("Groq API failed:", err); + const providerError = err as { + message?: string; + status?: number; + code?: string; + response?: { status?: number }; + }; + console.error("Groq API failed:", { + message: providerError?.message, + status: providerError?.status ?? providerError?.response?.status, + code: providerError?.code, + }); return buildAiProviderErrorResponse(err); }🤖 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/check-similarity/route.ts` around lines 112 - 114, The catch block that currently does console.error("Groq API failed:", err) should be changed to log only a sanitized subset of the error (e.g., const safeErr = { message: err?.message, status: err?.status || err?.statusCode, code: err?.code }) and then call console.error("Groq API failed:", safeErr); keep returning buildAiProviderErrorResponse(err) unchanged; update the catch in the same async route handler (the catch that references buildAiProviderErrorResponse) to construct and log safeErr instead of the raw err.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/api/doubts/check-similarity/route.ts`:
- Around line 112-114: The catch block that currently does console.error("Groq
API failed:", err) should be changed to log only a sanitized subset of the error
(e.g., const safeErr = { message: err?.message, status: err?.status ||
err?.statusCode, code: err?.code }) and then call console.error("Groq API
failed:", safeErr); keep returning buildAiProviderErrorResponse(err) unchanged;
update the catch in the same async route handler (the catch that references
buildAiProviderErrorResponse) to construct and log safeErr instead of the raw
err.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0f294feb-4c88-4394-bb66-5f109cbf94f0
📒 Files selected for processing (7)
src/__tests__/api/ai-career-chat-agent.test.tssrc/__tests__/api/doubts-similarity.test.tssrc/__tests__/lib/ai-kill-switch.test.tssrc/app/api/ai-career-chat-agent/route.tsxsrc/app/api/doubts/check-similarity/route.tssrc/lib/ai/kill-switch.tssrc/lib/request-identity.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/api/ai-career-chat-agent/route.tsx
- src/lib/ai/kill-switch.ts
User description
Closes #508
Summary
AI_ENABLEDkill switch shared by the requested AI routesAI_ENABLEDandAI_DAILY_USER_LIMITand add focused control/error testsValidation
git diff --checkCodeAnt-AI Description
Add AI usage controls and return clearer AI errors
What Changed
Impact
✅ Lower AI usage charges✅ Clearer AI limit and outage messages✅ Fewer empty similarity-check results💡 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
New Features
Improvements