Daily branch 2026 01 09 #158
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR gates the Agents settings tab by subscription, adds Redis Pub/Sub for stream cancellation with a polling fallback, converts web-search recency to a string enum, introduces model-aware pricing in rate-limiting, disables XAI conversation storage in a few generation calls, updates providers/dependencies, and adds Redis-related utilities and Convex publish action. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Browser
participant ChatHandler as chat-handler
participant Convex as Convex Backend
participant Redis as Redis Pub/Sub
participant Subscriber as CancellationSubscriber
Note over Client,Convex: Stream lifecycle with Redis cancellation
Client->>ChatHandler: start stream (chat request)
ChatHandler->>Convex: begin streaming mutation
ChatHandler->>Subscriber: createCancellationSubscriber(chatId)
Subscriber->>Redis: connect & subscribe to channel stream:cancel:{chatId}
Redis-->>Subscriber: subscription ready
par Streaming
Convex-->>ChatHandler: stream chunks
ChatHandler-->>Client: forward chunks
and Cancellation path
Client->>Convex: cancel request (publishCancellation(chatId))
Convex->>Redis: PUBLISH stream:cancel:{chatId} {"canceled":true}
Redis->>Subscriber: deliver cancel message
Subscriber->>ChatHandler: invoke onStop / abort
ChatHandler->>Convex: stop/cleanup stream
ChatHandler->>Subscriber: stop()
Subscriber->>Redis: unsubscribe & disconnect
end
sequenceDiagram
autonumber
participant Client as Browser
participant ChatHandler as chat-handler
participant RateLimit as checkRateLimit
participant TokenBucket as token-bucket
participant Usage as deductAgentUsage
Note over Client,Usage: Model-aware rate limiting flow
Client->>ChatHandler: send chat request (mode, subscription, modelName)
ChatHandler->>RateLimit: checkRateLimit(..., mode, subscription, estTokens, modelName)
alt Ask mode
RateLimit->>TokenBucket: ask-specific checks
else Agent mode
RateLimit->>TokenBucket: checkAgentRateLimit(..., modelName)
TokenBucket->>TokenBucket: select pricing (AGENT_VISION_PRICING vs DEFAULT_PRICING)
TokenBucket-->>RateLimit: allowed/denied
end
RateLimit-->>ChatHandler: decision
ChatHandler->>Usage: deductAgentUsage(..., actualInput, actualOutput, modelName)
Usage->>TokenBucket: calculateTokenCost(..., modelName)
TokenBucket-->>Usage: cost computed
Usage->>Usage: record usage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🧹 Nitpick comments (1)
app/components/SettingsDialog.tsx (1)
41-64: LGTM! Agents tab correctly gated by subscription tier.The logic properly restricts the Agents tab to paid subscriptions (Pro, Ultra, and Team tiers) while keeping it hidden from free users. The conditional tab assembly is clear and correct.
♻️ Optional: Group all paid-tier tabs for consistency
For improved readability, consider grouping all paid-tier tabs together:
- // Base tabs visible to all users const baseTabs = [ { id: "Personalization", label: "Personalization", icon: Settings }, { id: "Security", label: "Security", icon: Shield }, { id: "Data controls", label: "Data controls", icon: Database }, ]; - // Tabs only for paid users (Pro/Ultra/Team) - const agentsTab = { id: "Agents", label: "Agents", icon: Infinity }; + // Tabs for paid users (Pro/Ultra/Team) + const paidTabs = { + agents: { id: "Agents", label: "Agents", icon: Infinity }, + localSandbox: { id: "Local Sandbox", label: "Local Sandbox", icon: Server }, + usage: { id: "Usage", label: "Usage", icon: ChartNoAxesCombined }, + }; + - const localSandboxTab = { - id: "Local Sandbox", - label: "Local Sandbox", - icon: Server, - }; - const usageTab = { id: "Usage", label: "Usage", icon: ChartNoAxesCombined }; const teamTab = { id: "Team", label: "Team", icon: Users }; const accountTab = { id: "Account", label: "Account", icon: CircleUserRound }; const tabs = subscription === "team" - ? [...baseTabs, agentsTab, localSandboxTab, usageTab, teamTab, accountTab] + ? [...baseTabs, ...Object.values(paidTabs), teamTab, accountTab] : subscription !== "free" - ? [...baseTabs, agentsTab, localSandboxTab, usageTab, accountTab] + ? [...baseTabs, ...Object.values(paidTabs), accountTab] : [...baseTabs, accountTab];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/SettingsDialog.tsxapp/components/chat.tsxlib/ai/tools/web-search.tslib/ai/tools/web.ts
🔇 Additional comments (1)
app/components/chat.tsx (1)
137-141: Clarify the rationale for changing the initial batch size from 15 to 14.The initial message batch size was reduced from 15 to 14, but no explanation is provided. Please document why this specific value was chosen—whether it's for performance, UI/UX considerations, or another reason.
…valid date errors
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/ai/tools/web.ts (1)
77-87: Consider explicit handling for the"all"recency value.The current logic works correctly because
recencyToDays["all"]returnsundefined, resulting in no date filter. However, this implicit behavior is subtle and could be made more explicit for maintainability.♻️ Optional: Make "all" handling explicit
// Calculate startPublishedDate based on recency enum const recencyToDays: Record<string, number> = { past_day: 1, past_week: 7, past_month: 30, past_year: 365, }; - const days = recency ? recencyToDays[recency] : undefined; + const days = recency && recency !== "all" ? recencyToDays[recency] : undefined; const startPublishedDate = days ? new Date(Date.now() - days * 24 * 60 * 60 * 1000).toISOString() : undefined;lib/ai/tools/web-search.ts (1)
47-57: Consider extracting sharedrecencyToDaysmapping.This mapping is duplicated in
lib/ai/tools/web.ts. For maintainability, consider extracting it to a shared constant or utility.♻️ Optional: Extract shared constant
Create a shared constant (e.g., in a common utility file):
// lib/ai/tools/shared.ts or similar export const RECENCY_TO_DAYS: Record<string, number> = { past_day: 1, past_week: 7, past_month: 30, past_year: 365, }; export type RecencyFilter = "all" | "past_day" | "past_week" | "past_month" | "past_year";Then import and use in both
web.tsandweb-search.ts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ai/tools/web-search.tslib/ai/tools/web.ts
🔇 Additional comments (4)
lib/ai/tools/web.ts (2)
39-44: LGTM! Enum-based recency improves API clarity.The change from numeric days to a semantic enum (
"all","past_day", etc.) makes the API more intuitive and self-documenting for AI tool consumers.
24-26: Good documentation enhancement.Adding search operator examples (site:, filetype:, quotes) helps users leverage advanced search capabilities effectively.
lib/ai/tools/web-search.ts (2)
17-17: LGTM! Description aligns with web.ts.The search operator guidance is consistent with the web tool, maintaining API documentation parity.
24-29: LGTM! Schema is consistent with the web tool.The enum-based recency schema matches
lib/ai/tools/web.ts, ensuring a unified API surface for both tools.
- Add Convex internal action for Redis publish with auth check - Create hybrid cancellation subscriber with Redis pub/sub + polling fallback - Integrate Redis publish into cancelStreamFromClient and cancelTempStreamFromClient - Remove separate API endpoint in favor of Convex scheduled action Benefits: - Instant cancellation when Redis is available (vs 1s polling delay) - Graceful fallback to 1s polling when Redis unavailable - Auth check enforced through Convex mutations
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @lib/utils/stream-cancellation.ts:
- Around line 159-171: The pub/sub abort handler (onAbort) currently only sets
stopped and calls cleanupSubscriber but omits invoking onStop(), causing
inconsistent behavior versus the polling path; update onAbort (used by
abortController.signal.addEventListener) to call onStop() (if defined) before or
alongside cleanupSubscriber, handling it as async and catching errors so
failures in onStop don't block cleanup; ensure you reference and update the
onAbort function and its invocation via abortController to await or handle
onStop() properly and preserve the existing once: true listener behavior.
- Around line 173-179: The pub/sub stop() implementation (the returned object's
stop method) doesn't remove the abort event listener, so save the handler
reference when you attach it (same pattern used in the polling version) and call
abortSignal.removeEventListener('abort', handler) inside stop() before or after
calling await cleanupSubscriber(); ensure you still set stopped = true and
preserve isUsingPubSub: true so the listener is fully detached if stop() is
invoked manually.
🧹 Nitpick comments (3)
lib/utils/redis-pubsub.ts (1)
10-28: Consider consistent error logging and clarify error handler behavior.The error handler on line 19 uses
console.error, while the connection failure on line 25 usesconsole.warn. For consistency, consider using the same log level for both Redis-related errors.Additionally, the error handler attached on line 19 remains active after connection, but there's no guidance on how calling code should handle runtime errors (e.g., connection drops). Consider documenting the expected behavior or providing a cleanup mechanism.
convex/redisPubsub.ts (1)
26-30: Consider adding a connection timeout and logging Redis client errors.The no-op error handler on line 28 silently swallows all Redis client errors, which can make debugging connection issues difficult. Additionally, there's no connection timeout configured, which could cause the action to hang if Redis is slow or unresponsive.
♻️ Suggested improvement
let client; try { - client = createClient({ url: redisUrl }); - client.on("error", () => {}); + client = createClient({ + url: redisUrl, + socket: { + connectTimeout: 5000, + }, + }); + client.on("error", (err) => { + console.warn("[Redis Pub/Sub] Client error:", err.message); + }); await client.connect();lib/utils/stream-cancellation.ts (1)
10-10: Use type-only import forcreateClient.The
createClientimport is only used for the type alias on line 13. Using a type-only import avoids including the runtime module unnecessarily.♻️ Suggested fix
-import { createClient } from "redis"; +import type { createClient } from "redis";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (7)
.env.local.exampleconvex/chats.tsconvex/redisPubsub.tsconvex/tempStreams.tslib/api/chat-handler.tslib/utils/redis-pubsub.tslib/utils/stream-cancellation.ts
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,js}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,js}: ALWAYS use the new function syntax for Convex functions withquery,mutation,actiondecorators that includeargs,returns, andhandlerproperties
Use array validators withv.array()to validate array arguments and schema fields
Use discriminated union types withv.union()andv.literal()for schema validation
Always use thev.null()validator when a function returns a null value
Usev.id(tableName)validator for document ID validation andId<'tableName'>TypeScript type for ID fields
v.bigint()is deprecated; usev.int64()instead for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes if you need different query orders
UseinternalQuery,internalMutation, andinternalActionfor private functions that should not be part of the public API
Usequery,mutation, andactionfor public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions includingquery,internalQuery,mutation,internalMutation,action, andinternalAction
Functions that don't return anything should includereturns: v.null()as the output validator
Functions implicitly returnnullin JavaScript if they have no return statement
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action
Only call an action from another action if you need to cross runtimes (e.g., V8 to Node); otherwise extract shared code into a helper async function
Use as few calls from actions to queries and mutations as possible to avoid race conditions from split transaction logic
Pass aFunctionReferencetoctx.runQuery,ctx.runMutation, orctx.runAction; do not pass the callee function directly
When usi...
Files:
convex/chats.tsconvex/redisPubsub.tsconvex/tempStreams.ts
🧠 Learnings (11)
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Use `internalQuery`, `internalMutation`, and `internalAction` for private functions that should not be part of the public API
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Function references for internal functions use the `internal` object from `convex/_generated/api.ts` with file-based routing (e.g., `internal.example.g` for function `g` in `convex/example.ts`)
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-08-27T12:38:06.662Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 14
File: convex/chats.ts:4-24
Timestamp: 2025-08-27T12:38:06.662Z
Learning: Convex functions with serviceKey parameters are designed for backend service-to-service communication and don't require ctx.auth.getUserIdentity() checks. The serviceKey validation against process.env.CONVEX_SERVICE_ROLE_KEY provides sufficient authentication for these backend operations.
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : ALWAYS include argument and return validators for all Convex functions including `query`, `internalQuery`, `mutation`, `internalMutation`, `action`, and `internalAction`
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 17
File: convex/fileStorage.ts:131-149
Timestamp: 2025-09-01T12:44:12.626Z
Learning: In convex/fileStorage.ts, serviceKey parameters are made optional (v.optional(v.string())) for queries like getFileTokensByFileIds to simplify local development workflow - developers don't need to configure service keys in the Convex dashboard when working locally. In production, the serviceKey is always provided, ensuring security. This pattern balances developer experience with production security requirements.
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Use `query`, `mutation`, and `action` for public functions that are exposed to the public API
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Paginated queries should use `paginationOptsValidator` from `convex/server` and call `.paginate(args.paginationOpts)` on the query chain
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` using `defineSchema` and `defineTable` imported from `convex/server`
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Function references for public functions use the `api` object from `convex/_generated/api.ts` with file-based routing (e.g., `api.example.f` for function `f` in `convex/example.ts`)
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Thoughtfully organize files with public query, mutation, or action functions within the `convex/` directory for file-based routing
Applied to files:
convex/tempStreams.ts
📚 Learning: 2025-12-24T18:09:08.574Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-12-24T18:09:08.574Z
Learning: Applies to convex/**/*.{ts,js} : Convex queries do NOT support `.delete()`; instead `.collect()` the results, iterate, and call `ctx.db.delete(row._id)` on each result
Applied to files:
convex/tempStreams.ts
🧬 Code graph analysis (2)
lib/api/chat-handler.ts (1)
lib/utils/stream-cancellation.ts (1)
createCancellationSubscriber(114-194)
lib/utils/stream-cancellation.ts (1)
lib/utils/redis-pubsub.ts (2)
getCancelChannel(33-35)createRedisSubscriber(10-28)
🔇 Additional comments (7)
.env.local.example (1)
83-83: LGTM! Helpful documentation addition.The comment guides users to configure REDIS_URL in the Convex Dashboard, which is essential for the new Redis Pub/Sub cancellation functionality to work in production.
lib/utils/redis-pubsub.ts (1)
33-35: LGTM!The channel naming convention is clear and the function is straightforward.
convex/tempStreams.ts (1)
78-81: LGTM! Non-blocking Redis notification correctly implemented.The scheduled Redis Pub/Sub notification is correctly placed after the database delete and runs asynchronously without blocking the mutation response. The lack of error handling is acceptable here since the cancellation has already been persisted by deleting the temp stream row, and the Redis notification is an optimization for instant backend notification.
Based on coding guidelines, the use of
internal.redisPubsub.publishCancellationcorrectly follows the pattern for calling internal functions.convex/chats.ts (1)
403-407: LGTM! Consistent implementation with temp streams.The Redis Pub/Sub notification follows the same pattern as in
convex/tempStreams.tsand is correctly placed after persisting the cancellation state. The non-blocking async execution ensures the mutation response isn't delayed.lib/api/chat-handler.ts (1)
181-190: LGTM! Well-integrated cancellation subscriber with proper cleanup.The refactoring from polling to Redis Pub/Sub-based cancellation with fallback is correctly implemented:
- The subscriber is awaited during initialization (line 183), ensuring it's ready before streaming begins
- The
onStopcallback correctly updates the state flag- Cleanup properly awaits
stop()(line 496) in the stream'sonFinishcallback, preventing Redis connection leaks- The comment on line 181 accurately describes the Redis pub/sub with fallback behavior
The changes integrate well with the broader Redis Pub/Sub cancellation infrastructure introduced in this PR.
Also applies to: 495-497
convex/redisPubsub.ts (1)
13-49: LGTM overall - proper Convex action structure.The implementation follows Convex coding guidelines correctly:
- Uses
internalActionfor private functionality- Includes proper
argsandreturnsvalidators- Has
"use node"directive at the top- Gracefully handles missing
REDIS_URLand errors- Ensures cleanup in the
finallyblocklib/utils/stream-cancellation.ts (1)
106-194: Good hybrid approach with graceful degradation.The implementation provides a solid pattern:
- Attempts Redis pub/sub for instant cancellation notifications
- Falls back gracefully to polling when Redis is unavailable
- Proper cleanup in the
cleanupSubscriberfunction- Error handling that doesn't break the cancellation flow
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/api/chat-handler.ts (1)
631-647: Critical: Cancellation subscriber is not cleaned up on error paths.The error handler clears the
preemptiveTimeoutbut does not stop thecancellationSubscribercreated at line 183. If an error occurs before the stream'sonFinishcallback executes, the subscriber will remain active, causing resource leaks:
- Redis Pub/Sub mode: Connection remains subscribed to the cancellation channel
- Polling fallback mode: Polling interval continues running indefinitely
This can accumulate over time with repeated errors, wasting Redis connections and/or CPU cycles.
🔧 Proposed fix: Add subscriber cleanup to error handler
} catch (error) { // Clear timeout if error occurs before onFinish preemptiveTimeout?.clear(); + + // Stop cancellation subscriber to prevent resource leaks + if (cancellationSubscriber) { + try { + await cancellationSubscriber.stop(); + } catch (cleanupError) { + console.error("Failed to stop cancellation subscriber:", cleanupError); + } + } // Handle ChatSDKErrors (including authentication errors)Note: You'll need to move the
cancellationSubscriberdeclaration outside the try block scope to make it accessible in the catch block:return async (req: NextRequest) => { let preemptiveTimeout: | ReturnType<typeof createPreemptiveTimeout> | undefined; + let cancellationSubscriber: + | Awaited<ReturnType<typeof createCancellationSubscriber>> + | undefined; try { // ... existing code ... - // Start cancellation subscriber (Redis pub/sub with fallback to polling) let subscriberStopped = false; - const cancellationSubscriber = await createCancellationSubscriber({ + cancellationSubscriber = await createCancellationSubscriber({
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/actions/index.tslib/api/chat-handler.tslib/utils/message-summarization.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/api/chat-handler.ts (1)
lib/utils/stream-cancellation.ts (1)
createCancellationSubscriber(114-194)
🔇 Additional comments (6)
lib/utils/message-summarization.ts (1)
156-161: LGTM! Consistent privacy improvement.The addition of
providerOptions.xai.store: falsealigns with the same change inlib/actions/index.tsand ensures conversations are not stored in XAI's database during summarization. The implementation is clean and well-commented.lib/actions/index.ts (1)
39-44: XAI provider option verified — LGTM!The addition of
providerOptions.xai.store: falsecorrectly prevents conversations from being stored in XAI's database. This is a valid privacy improvement supported by the xAI API. The implementation is consistent across bothlib/actions/index.tsandlib/utils/message-summarization.ts.lib/api/chat-handler.ts (4)
37-37: LGTM: Import updated for new cancellation mechanism.The import change from
createCancellationPollertocreateCancellationSubscriberaligns with the migration to Redis Pub/Sub-based cancellation with polling fallback.
181-190: LGTM: Subscriber creation properly migrated to async pattern.The cancellation mechanism has been correctly updated to use the new subscriber-based approach:
- Properly awaits the async
createCancellationSubscribercall- Variable naming updated consistently from
pollerStoppedtosubscriberStopped- onStop callback correctly updates the subscriber state
The implementation gracefully falls back to polling when Redis is unavailable.
409-412: LGTM: Privacy improvement for XAI provider.Disabling conversation storage in XAI's database is a good privacy measure. This configuration ensures user conversations are not persisted externally.
499-501: LGTM: Subscriber cleanup properly awaited.The subscriber stop call is correctly awaited in the stream's onFinish callback, ensuring proper cleanup of Redis resources.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @package.json:
- Around line 44-46: The dependency bumps for @ai-sdk/react (@ai-sdk/react
3.0.13→3.0.26), @ai-sdk/openai, @ai-sdk/xai and ai (6.0.13→6.0.25) may introduce
breaking changes affecting cancellation and web-search recency behavior; run the
full CI/test pipeline and add/execute focused tests that exercise your
cancellation logic (e.g., cancelRequest/cancelToken flows) and web-search
recency handling (e.g., performWebSearch/recencyCheck integration paths), verify
the lockfile is committed and repeatable, and if any failures appear either pin
the working versions in package.json or update code to match the new SDK APIs
and re-run CI until all tests pass.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
lib/utils/message-summarization.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/utils/message-summarization.ts
🧰 Additional context used
📓 Path-based instructions (1)
package.json
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
Always add
@types/nodeto yourpackage.jsonwhen using any Node.js built-in modules
Files:
package.json
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ai/providers.ts (1)
28-28: Fix inconsistent cutoff date for agent-vision-model.The
agent-vision-modelnow usesgoogle/gemini-3-flash-preview(same asask-vision-model), but has a cutoff date of "November 2024" whileask-vision-modelusing the identical model has "January 2025" (line 25). This inconsistency will provide users with incorrect knowledge cutoff information.📅 Proposed fix for cutoff date consistency
"agent-model": "November 2024", - "agent-vision-model": "November 2024", + "agent-vision-model": "January 2025", "title-generator-model": "November 2024",
🤖 Fix all issues with AI agents
In @lib/ai/providers.ts:
- Line 14: The cutoff date for agent-vision-model is inconsistent: update the
cutoff date for "agent-vision-model" (currently set to "November 2024") to match
the model's definition used by "ask-vision-model" by changing its cutoff date to
"January 2025"; locate the agent-vision-model entry (the line with
"agent-vision-model": openrouter("google/gemini-3-flash-preview")) and update
its cutoffDate value to "January 2025" so both entries using
google/gemini-3-flash-preview share the same cutoff.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ai/providers.tslib/system-prompt.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T13:31:08.519Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 21
File: app/api/chat/route.ts:121-128
Timestamp: 2025-09-06T13:31:08.519Z
Learning: The vision model (NEXT_PUBLIC_VISION_MODEL) used for processing media files also supports tool calling, so overriding to vision-model in Agent mode with media files does not break tool functionality.
Applied to files:
lib/ai/providers.ts
🔇 Additional comments (1)
lib/system-prompt.ts (1)
154-157: LGTM! Clean refactoring.The extraction of product questions handling into a reusable helper function follows the DRY principle and improves maintainability. The function is consistently applied in both ask and agent modes.
- Add AGENT_VISION_PRICING ($0.50/1M input, $3.00/1M output) - Update calculateTokenCost to check modelName for pricing selection - Update checkAgentRateLimit and deductAgentUsage to accept modelName - Reorder chat-handler flow: ask mode checks rate limit first, agent mode processes first to determine model - Simplify rate limit flow with null coalescing pattern
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @lib/api/benchmark-handler.ts:
- Around line 165-176: The current try/catch around
handleInitialChatAndUserMessage quietly swallows failures which contradicts the
"Always create chat for benchmark runs" intent; update the error handling so
chat creation is treated as a hard failure (or at minimum logged as an error and
rethrown) — replace the console.warn in the catch with an error-level log (e.g.,
processLogger.error or throw the caught error) and ensure any upstream caller
will stop the benchmark run if handleInitialChatAndUserMessage({ chatId, userId,
messages: inputMessages, regenerate: false, chat: null }) fails.
- Around line 68-87: The validateBenchmarkApiKey function currently uses direct
equality (providedKey === expectedKey); replace this with a timing-safe
comparison using crypto.timingSafeEqual: import Node's crypto, convert both
providedKey and expectedKey to Buffers (e.g., Buffer.from(..., "utf8")), first
check that the buffers have identical lengths (return false if not) and then
return crypto.timingSafeEqual(bufProvided, bufExpected); keep all existing
header parsing logic (Bearer support) and the early checks for missing
expectedKey/authHeader.
🧹 Nitpick comments (3)
lib/api/chat-handler.ts (1)
189-196:subscriberStoppedvariable is declared but never read.The
subscriberStoppedflag is set in multiple places (lines 189, 195, 509) but is never used in any conditional logic. Consider removing it if unused, or document its intended purpose if kept for debugging.♻️ If unused, consider removing
- let subscriberStopped = false; const cancellationSubscriber = await createCancellationSubscriber({ chatId, isTemporary: !!temporary, abortController: userStopSignal, - onStop: () => { - subscriberStopped = true; - }, + onStop: () => {}, });And at line 509:
await cancellationSubscriber.stop(); - subscriberStopped = true;lib/api/benchmark-handler.ts (2)
41-63: DuplicategetStreamContextimplementation withchat-handler.ts.This function is nearly identical to the one in
lib/api/chat-handler.ts. Consider extracting it to a shared utility (e.g.,lib/utils/stream-context.ts) to avoid duplication and ensure consistent behavior.♻️ Example extraction
Create
lib/utils/stream-context.ts:import { createResumableStreamContext } from "resumable-stream"; import { after } from "next/server"; let globalStreamContext: ReturnType<typeof createResumableStreamContext> | null = null; export const getStreamContext = () => { if (!globalStreamContext) { try { globalStreamContext = createResumableStreamContext({ waitUntil: after }); } catch (error: unknown) { if ( typeof (error as Error)?.message === "string" && (error as Error).message.includes("REDIS_URL") ) { console.log(" > Resumable streams are disabled due to missing REDIS_URL"); } else { console.warn("Resumable stream context init failed:", error); } } } return globalStreamContext; };Then import from both handlers.
248-253: Missingxai.store: falseprovider option.Unlike
chat-handler.ts, this handler doesn't setproviderOptions.xai.store: false. For consistency and privacy, consider adding it here as well.♻️ Add XAI store option
providerOptions: { + xai: { + store: false, + }, openrouter: { reasoning: { enabled: true }, provider: { sort: "latency" }, }, },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
lib/ai/providers.tslib/api/benchmark-handler.tslib/api/chat-handler.tslib/rate-limit/index.tslib/rate-limit/token-bucket.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-06T13:31:08.519Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 21
File: app/api/chat/route.ts:121-128
Timestamp: 2025-09-06T13:31:08.519Z
Learning: The vision model (NEXT_PUBLIC_VISION_MODEL) used for processing media files also supports tool calling, so overriding to vision-model in Agent mode with media files does not break tool functionality.
Applied to files:
lib/rate-limit/token-bucket.tslib/ai/providers.ts
🧬 Code graph analysis (4)
lib/rate-limit/index.ts (1)
lib/rate-limit/token-bucket.ts (1)
checkAgentRateLimit(147-226)
lib/api/benchmark-handler.ts (9)
lib/api/chat-handler.ts (1)
getStreamContext(60-78)lib/errors.ts (1)
ChatSDKError(36-74)types/chat.ts (1)
Todo(62-67)lib/chat/chat-processor.ts (2)
processChatMessages(103-144)getMaxStepsForUser(12-32)lib/db/actions.ts (1)
handleInitialChatAndUserMessage(105-166)lib/ai/tools/index.ts (1)
createTools(33-134)lib/ai/providers.ts (1)
createTrackedProvider(41-69)lib/system-prompt.ts (1)
systemPrompt(241-306)lib/utils/message-processor.ts (1)
stripProviderMetadata(61-69)
lib/rate-limit/token-bucket.ts (1)
lib/rate-limit/index.ts (1)
calculateTokenCost(22-22)
lib/api/chat-handler.ts (3)
lib/rate-limit/index.ts (1)
checkRateLimit(53-70)lib/token-utils.ts (1)
countMessagesTokens(120-129)lib/utils/stream-cancellation.ts (1)
createCancellationSubscriber(114-194)
🔇 Additional comments (15)
lib/ai/providers.ts (2)
28-28: The cutoff date forgoogle/gemini-3-flash-previewis correctly set to January 2025.
14-14: Confirm monitoring and fallback strategy for preview model in production.The switch to
google/gemini-3-flash-previewis verified to support tool calling in agent mode. However, this model carries preview status with shorter deprecation windows and stricter rate limits. Ensure you have:
- Monitoring and alerting for API changes or unexpected behavior
- A fallback strategy to a stable Gemini model if needed
- Clear deprecation timeline awareness for this preview variant
The January 2025 knowledge cutoff is accurate.
⛔ Skipped due to learnings
Learnt from: RostyslavManko Repo: hackerai-tech/hackerai PR: 21 File: app/api/chat/route.ts:121-128 Timestamp: 2025-09-06T13:31:08.519Z Learning: The vision model (NEXT_PUBLIC_VISION_MODEL) used for processing media files also supports tool calling, so overriding to vision-model in Agent mode with media files does not break tool functionality.lib/rate-limit/index.ts (1)
50-66: LGTM! Model-aware pricing parameter propagation is correct.The new
modelNameparameter is properly documented, defaults to an empty string (consistent withtoken-bucket.ts), and is correctly passed through tocheckAgentRateLimitonly for agent mode. Ask mode appropriately doesn't require model-specific pricing.lib/api/chat-handler.ts (5)
37-39: LGTM! Cancellation mechanism upgraded to Redis Pub/Sub.The import change from
createCancellationPollertocreateCancellationSubscriberaligns with the new pub/sub-based cancellation approach with polling fallback.
149-172: LGTM! Two-stage rate limit approach is well-structured.The restructured flow correctly:
- Checks ask-mode rate limits early (before model selection) since ask mode doesn't need model-specific pricing
- Defers agent-mode rate limit checks until after
selectedModelis known for accurate model-aware pricing- Uses nullish coalescing to cleanly select the appropriate result
416-419: LGTM! Privacy-conscious XAI configuration.Disabling conversation storage in XAI's database is a good privacy practice. This is consistent with similar changes in
lib/actionsandlib/utils/message-summarization.tsmentioned in the AI summary.
484-492: LGTM! Model-aware usage deduction.Correctly passes
selectedModeltodeductAgentUsagefor accurate model-specific cost calculations, aligning with the updatedtoken-bucket.tssignature.
507-509: LGTM! Proper async cleanup of cancellation subscriber.The subscriber is properly awaited during cleanup, ensuring Redis connections are closed gracefully.
lib/rate-limit/token-bucket.ts (4)
11-23: LGTM! Model-specific pricing configuration is clear and well-organized.The pricing structure clearly separates:
DEFAULT_PRICING(Grok 4.1) with long-context variantsAGENT_VISION_PRICINGwith higher input/output costsThe pricing values appear reasonable for vision model workloads.
147-183: LGTM! Model name propagation in checkAgentRateLimit is correct.The
modelNameparameter is properly added to the function signature and correctly passed through tocalculateTokenCostfor estimated input cost calculation.
231-270: LGTM! Consistent model name propagation in deductAgentUsage.All three
calculateTokenCostcalls (estimated input, actual input, and output) correctly receive themodelNameparameter, ensuring consistent pricing across the entire usage calculation.
45-66: No changes needed — the model identifier is correctly configured.The hardcoded string
"agent-vision-model"in the pricing logic matches the actual model identifier defined inlib/ai/providers.tsand is consistently used throughout the codebase. The pricing logic correctly applies different rates for agent vision mode and appropriately restricts long-context pricing to non-vision models only.lib/api/benchmark-handler.ts (3)
255-264: Verify log sensitivity for security benchmarks.The
onStepFinishlogs include tool names and partial text output. For security benchmarks, this could potentially log sensitive test data. Consider if this logging level is appropriate for production, or if it should be gated behind a debug flag.
337-370: LGTM! Comprehensive error handling.The error handling covers:
- ChatSDKError for domain errors
- AbortError for timeouts with appropriate 408 status
- Unexpected errors wrapped for consistent response format
The timeout cleanup properly clears
timeoutIdin all error paths.
137-149: This endpoint is not currently exposed. ThecreateBenchmarkHandler()function exists but is not imported or used anywhere in the codebase, and no route file exposes it as an API endpoint. Rate limiting considerations should be addressed when/if this endpoint is integrated and made public.If this endpoint is intended for production use, consider adding request-based rate limiting (e.g., per-IP or per-key) before exposure, since the endpoint bypasses normal user rate limits and forces
subscription = "ultra". Currently, protection relies solely on the BENCHMARK_API_KEY environment variable.Likely an incorrect or invalid review comment.
Ensures consistent behavior between polling and pub/sub cancellation paths. Previously, external aborts (e.g., pre-emptive timeout, client disconnect) would not invoke onStop() for pub/sub subscribers.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/utils/stream-cancellation.ts (1)
6-13: UseRedisClientTypetype-only import instead of deriving fromcreateClient.
import { createClient } from "redis";is only used to derive the type viaReturnType<typeof createClient>. In redis@5.10.0, theRedisClientTypetype is available as a dedicated export and can be imported as a type-only import, avoiding unnecessary runtime bundling:import type { RedisClientType } from "redis"; type RedisClient = RedisClientType;This prevents runtime code for
createClientfrom being pulled into edge/serverless bundles.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/utils/stream-cancellation.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/utils/stream-cancellation.ts (1)
lib/utils/redis-pubsub.ts (2)
getCancelChannel(33-35)createRedisSubscriber(10-28)
🔇 Additional comments (1)
lib/utils/stream-cancellation.ts (1)
30-33:createCancellationPollerreturn-shape unification is correct;stop()does not invokeonStop()by design.Confirmed:
stop()is a cleanup method that does not triggeronStop(). TheonStop()callback runs only when cancellation is detected (via Redis pub/sub message at line 151 or external abort at line 162 in the subscriber path, and via abort event at line 85 in the poller path). The asyncstop()method andisUsingPubSubboolean flag simplify the caller contract as intended. Callers like chat-handler.ts correctly passonStop()for internal state tracking (line 194-196) while callingstop()separately for cleanup (line 508).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/api/chat-handler.ts (1)
149-174: Verify the sequencing of rate limit checks for AGENT mode.The two-phase rate limiting approach checks ASK mode before
processChatMessages(line 150-153) but delays AGENT mode checks until after model selection (line 166-174). This means AGENT mode users who are rate-limited still incur the cost of message processing, model selection, and sandbox file handling.Consider whether an early coarse-grained check for AGENT mode would improve resource utilization, or confirm this sequencing is intentional for UX/business reasons.
Potential optimization: Early AGENT mode rate limit check
// Ask mode: check rate limit first (avoid processing if over limit) const askRateLimitInfo = mode === "ask" ? await checkRateLimit(userId, mode, subscription) : null; + +// Agent mode: perform early check without model info (coarse-grained) +const earlyAgentCheck = + mode === "agent" + ? await checkRateLimit(userId, mode, subscription, 0) + : null; const { processedMessages, selectedModel, sandboxFiles } = await processChatMessages({ messages: truncatedMessages, mode, subscription, }); // Agent mode: check rate limit with model-specific pricing after knowing the model const estimatedInputTokens = mode === "agent" ? countMessagesTokens(truncatedMessages, {}) : 0; const rateLimitInfo = askRateLimitInfo ?? + earlyAgentCheck ?? (await checkRateLimit( userId, mode, subscription, estimatedInputTokens, selectedModel, ));
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/api/chat-handler.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/api/chat-handler.ts (3)
lib/rate-limit/index.ts (1)
checkRateLimit(53-70)lib/token-utils.ts (1)
countMessagesTokens(120-129)lib/utils/stream-cancellation.ts (1)
createCancellationSubscriber(114-195)
🔇 Additional comments (5)
lib/api/chat-handler.ts (5)
37-37: LGTM: Import updated for Redis Pub/Sub migration.The import change from
createCancellationPollertocreateCancellationSubscriberis consistent with the architectural shift to Redis Pub/Sub messaging for stream cancellation, with polling as a fallback.
190-199: LGTM: Cancellation subscriber correctly initialized.The migration to
createCancellationSubscriberis properly implemented withawait, and the variable naming (subscriberStopped) is consistent throughout the codebase. The function gracefully falls back to polling when Redis Pub/Sub is unavailable.
509-511: LGTM: Cancellation subscriber cleanup is properly handled.The subscriber stop operation is correctly awaited, and the flag update maintains consistency with the earlier initialization. The cleanup properly handles async Redis unsubscribe operations.
486-495: No changes needed—deductAgentUsagefunction signature correctly accepts the model parameter.The function at
lib/rate-limit/token-bucket.ts(line 231) acceptsmodelName = ""as its sixth parameter, which correctly receivesselectedModelfrom the call site. The function signature has been properly updated to support model-aware pricing calculations.
418-421: Thexai.store: falseoption is supported and correctly implemented.The
storeproperty is a documented feature in the @ai-sdk/xai provider that controls whether responses are persisted in XAI's database. Setting it tofalseis the correct approach to disable persistence, and the implementation matches the documented API.
…nup races - Add onStopCalled flag to ensure onStop() is only invoked once - Remove abort event listener in .stop() to prevent unexpected callbacks - Make subscribe callback synchronous to avoid race with quit() - Use fire-and-forget cleanup pattern for Redis unsubscribe/quit - Make abort handler the single source of truth for cleanup
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/utils/stream-cancellation.ts (1)
10-13: Consider importingRedisClienttype fromredis-pubsub.tsinstead of duplicating.The comment acknowledges this should match
redis-pubsub.ts, but duplicating the type definition risks drift. Consider exportingRedisClientfromredis-pubsub.tsand importing it here.♻️ Suggested refactor
In
lib/utils/redis-pubsub.ts, export the type:export type RedisClient = ReturnType<typeof createClient>;Then here:
import { createRedisSubscriber, getCancelChannel, + RedisClient, } from "@/lib/utils/redis-pubsub"; -import { createClient } from "redis"; - -// Use the same type as redis-pubsub.ts -type RedisClient = ReturnType<typeof createClient>;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/utils/stream-cancellation.ts
🧰 Additional context used
🧬 Code graph analysis (1)
lib/utils/stream-cancellation.ts (1)
lib/utils/redis-pubsub.ts (2)
getCancelChannel(33-35)createRedisSubscriber(10-28)
🔇 Additional comments (2)
lib/utils/stream-cancellation.ts (2)
93-103: Verify:stop()doesn't callonStopcallback, unlike abort path.In
createCancellationPoller, theonAborthandler callsonStop()(line 85), but the manualstop()method (lines 94-101) does not. This creates behavioral asymmetry compared tocreateCancellationSubscriberwherestop()also doesn't callcallOnStopOnce().If this is intentional (i.e.,
stop()is for cleanup without triggering the callback), consider adding a doc comment to clarify. If not, thestop()method should also invokeonStop.
114-198: Well-structured hybrid cancellation implementation.The implementation correctly:
- Uses
callOnStopOnce()guard to prevent duplicate callback invocations- Implements fire-and-forget cleanup with swallowed errors for
unsubscribe/quit- Handles JSON parse errors defensively
- Falls back gracefully to polling when Redis is unavailable
- Uses
{once: true}for the abort listener to prevent memory leaks
Summary by CodeRabbit
New Features
New Features / Enhancements
Changes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.