Daily branch 2025 09 28 #40
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds service-role file upload support and tooling to fetch sandbox files, accumulate uploaded file IDs, and persist them on assistant messages. Updates chat route to attach saved file IDs, adjusts model selection for PDFs, revises providers, renames Convex message mutations, enhances UI to preview saved files, and improves reasoning scroll. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Chat UI
participant API as Chat Route
participant TL as Tools (createTools)
participant SB as Sandbox
participant UPL as Sandbox Uploader
participant CX as Convex (fileStorage/fileActions)
participant DB as DB
U->>FE: Run terminal task
FE->>API: Send message
API->>TL: Initialize tools + getFileAccumulator
TL->>SB: Execute run_terminal_cmd
SB-->>TL: stdout/stderr, files present
TL->>TL: get_terminal_files (paths)
TL->>UPL: uploadSandboxFileToConvex(path, userId)
UPL->>CX: fileStorage.generateUploadUrl(serviceKey, userId)
CX-->>UPL: upload URL
UPL->>CX: POST blob → fileActions.saveFile(...)
CX-->>UPL: { url, fileId, tokens }
UPL-->>TL: UploadedFileInfo
TL->>TL: fileAccumulator.add(fileId) %% new/changed
TL-->>API: Tool result with fileUrls
API->>TL: getFileAccumulator().getAll() %% new/changed
API->>DB: saveMessage(..., extraFileIds=[...]) %% new/changed
DB-->>API: ok
API-->>FE: Assistant message + saved file URLs
FE-->>U: Render message and preview cards with links
sequenceDiagram
autonumber
participant CP as Chat Processor
participant MS as Model Selector
CP->>MS: selectModel(mode, containsMedia, containsPdf)
alt containsPdf && mode=="ask" %% new branch
MS-->>CP: vision-model-for-pdfs
else containsMedia && mode=="ask"
MS-->>CP: vision-model
else containsMedia && mode=="agent"
MS-->>CP: agent-model-with-vision
else
MS-->>CP: default by mode
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
convex/messages.ts (1)
315-334: Avoid .filter(); switch to an index on (chat_id, role) to fetch the last assistant message.Per Convex guidelines, prefer index-backed predicates over filter to avoid scanning. Define an index like by_chat_id_role(chat_id, role, _creationTime) and use it here.
Apply after adding the index to your schema:
- const lastAssistantMessage = await ctx.db - .query("messages") - .withIndex("by_chat_id", (q) => q.eq("chat_id", args.chatId)) - .filter((q) => q.eq(q.field("role"), "assistant")) - .order("desc") - .first(); + const lastAssistantMessage = await ctx.db + .query("messages") + .withIndex("by_chat_id_role", (q) => + q.eq("chat_id", args.chatId).eq("role", "assistant"), + ) + .order("desc") + .first();As per coding guidelines.
lib/db/actions.ts (1)
50-60: Use Id<'files'> for extraFileIds and enforce deduplication end-to-end
- In lib/db/actions.ts (lines 50–60): change
extraFileIds?: string[];
to
extraFileIds?: Array<Id<"files">>;
and add
import type { Id } from "@/convex/_generated/dataModel";- In lib/db/actions.ts (lines 64–67): replace the current array merge with
then remove anyconst mergedFileIds: Array<Id<"files">> = Array.from( new Set<Id<"files">>([ ...fileIds, ...(extraFileIds ?? []), ]), );as anycasts when assigningfileIds: mergedFileIds || undefined.- In app/api/chat/route.ts (lines 253–259): update both calls to
saveMessageso thatextraFileIdsis passed asArray<Id<"files">>(e.g. castnewFileIdstoArray<Id<"files">>instead ofstring[]).app/hooks/useChatHandlers.ts (1)
80-82: Prevent sending an empty message when files lack fileIdhasValidFiles allows submission if a file has url but no fileId; later validFiles filters out those without fileId, producing an empty payload (no text, no files).
Apply:
- const hasValidFiles = uploadedFiles.some((f) => f.uploaded && f.url); + const hasValidFiles = uploadedFiles.some( + (f) => f.uploaded && f.url && f.fileId + );Optionally, fail fast with a toast if files exist but are missing IDs.
Also applies to: 106-123
convex/fileActions.ts (1)
179-187: Do not trust client-provided mediaType; use Blob.type/detection to prevent bypass.Currently, an attacker can pass mediaType="image/png" for a non-image file to force 0 tokens and skip content persistence. The same issue affects shouldSaveContent, which relies on args.mediaType. Gate on the actual Blob.type (and/or detected type) instead.
Apply this diff to base decisions on the real file MIME and drop reliance on args.mediaType in these hotspots:
@@ - // Check if file is an image - return 0 tokens immediately - if (mediaType && isImageFile(mediaType)) { + // Check if file is an image using the actual blob MIME + const blobMime = + typeof file !== "string" ? (file as Blob).type : undefined; + if (blobMime && isImageFile(blobMime)) { return [ { content: "", tokens: 0, }, ]; } @@ - // Check if file is an image - return 0 tokens - if (mediaType && isImageFile(mediaType)) { + // Check if file is an image - return 0 tokens + const blobMime = + typeof file !== "string" ? (file as Blob).type : undefined; + if (blobMime && isImageFile(blobMime)) { return [ { content: "", tokens: 0, }, ]; - } else if (mediaType && mediaType.startsWith("text/")) { + } else if (blobMime?.startsWith("text/")) { try { const blob = file as Blob; const fileBuffer = Buffer.from(await blob.arrayBuffer()); const textDecoder = new TextDecoder("utf-8"); const textContent = textDecoder.decode(fileBuffer); const fallbackTokens = countTokens(textContent); @@ - // Save content for non-image, non-PDF, non-binary files - const shouldSaveContent = - !isImageFile(args.mediaType) && - args.mediaType !== "application/pdf" && - chunks.length > 0 && - chunks[0].content.length > 0; + // Save content for non-image/non-PDF when we actually have content + const blobMime = file.type; + const hasContent = chunks.some((c) => c.content.length > 0); + const shouldSaveContent = + hasContent && + !(blobMime?.startsWith("image/")) && + blobMime !== "application/pdf";Also applies to: 216-261, 418-423
🧹 Nitpick comments (21)
.env.local.example (1)
6-10: Avoid shipping an “active” placeholder for CONVEX_SERVICE_ROLE_KEY; fix ordering and linter noise.
- Keep the key empty or commented to prevent accidental reuse in real deployments.
- Move it before NEXT_PUBLIC_CONVEX_URL per dotenv-linter and drop quotes to silence QuoteCharacter.
Apply:
-# Convex Service Role Key (Required for securing public functions) -# Generate a secure random string and add it to your Convex environment variables -CONVEX_SERVICE_ROLE_KEY="replace-with-a-32+char-random-secret" -NEXT_PUBLIC_CONVEX_URL= +# Convex Service Role Key (Required for securing public functions) +# Generate a secure random string and add it to your Convex environment variables +# CONVEX_SERVICE_ROLE_KEY=replace-with-a-32+char-random-secret +NEXT_PUBLIC_CONVEX_URL=Note: Ensure this key is only used server-side and never referenced from client bundles. Based on learnings.
app/components/ReasoningHandler.tsx (1)
55-56: LGTM; optional naming alignment.Change reads clean and preserves behavior. If you want absolute consistency with the prop, consider naming the local boolean isStreaming.
Also applies to: 59-59
lib/ai/providers.ts (1)
33-36: Tighten types for trackedModels to match provider expectations.Use LanguageModelV1 instead of any to prevent drift and surface type errors at compile time.
Apply:
-import type { SubscriptionTier } from "@/types"; +import type { SubscriptionTier } from "@/types"; +import type { LanguageModelV1 } from "ai"; - const trackedModels: Record<string, any> = {}; + const trackedModels: Record<string, LanguageModelV1> = {};lib/ai/tools/utils/file-accumulator.ts (1)
3-21: Add a drain helper and explicit return types for clarity and safer lifecycle.
- Returning types helps callers.
- Providing a
drain()(get-and-clear) reduces misuse where callers forget to callclear()aftergetAll().export class FileAccumulator { - private ids: Set<Id<"files">> = new Set(); + private ids: Set<Id<"files">> = new Set(); - add(id: Id<"files">) { + add(id: Id<"files">): void { this.ids.add(id); } - addMany(ids: Array<Id<"files">>) { + addMany(ids: Array<Id<"files">>): void { for (const id of ids) this.ids.add(id); } - getAll(): Array<Id<"files">> { + getAll(): Array<Id<"files">> { return Array.from(this.ids); } - clear() { + clear(): void { this.ids.clear(); } + + // Atomically get and clear to avoid duplicate attachments across saves + drain(): Array<Id<"files">> { + const all = this.getAll(); + this.clear(); + return all; + } }types/agent.ts (1)
5-6: Mark FileAccumulator import as type-only to avoid unintended runtime dependency.Prevents bundlers from pulling the module at runtime for a type-only usage.
-import { FileAccumulator } from "@/lib/ai/tools/utils/file-accumulator"; +import type { FileAccumulator } from "@/lib/ai/tools/utils/file-accumulator";Also applies to: 25-26
components/ai-elements/reasoning.tsx (1)
53-64: Use a single Collapsible root for proper a11y semantics and simpler state wiring.Two separate Collapsible roots (one in Trigger, one in Content) can desync aria-controls relationships. Wrap children with one Collapsible in
Reasoning, and drop the inner wrappers. This keeps controlled open state and improves accessibility.@@ - return ( - <ReasoningContext.Provider - value={{ isOpen: !!isOpen, setIsOpen, isStreaming }} - > - <div - className={cn("not-prose max-w-prose space-y-2", className)} - {...props} - > - {children} - </div> - </ReasoningContext.Provider> - ); + return ( + <ReasoningContext.Provider value={{ isOpen: !!isOpen, setIsOpen, isStreaming }}> + <Collapsible open={!!isOpen} onOpenChange={setIsOpen}> + <div className={cn("not-prose max-w-prose space-y-2", className)} {...props}> + {children} + </div> + </Collapsible> + </ReasoningContext.Provider> + ); @@ - return ( - <Collapsible onOpenChange={setIsOpen} open={isOpen}> - <CollapsibleTrigger + return ( + <CollapsibleTrigger className={cn( "flex w-full items-center gap-2 text-muted-foreground text-sm transition-colors hover:text-foreground", className, )} aria-label={title} {...props} > @@ - </CollapsibleTrigger> - </Collapsible> + </CollapsibleTrigger> ); @@ - return ( - <Collapsible open={isOpen}> - <CollapsibleContent + return ( + <CollapsibleContent className={cn( "mt-2 space-y-3", "data-[state=closed]:fade-out-0 data-[state=closed]:slide-out-to-top-2 data-[state=open]:slide-in-from-top-2 text-popover-foreground outline-none data-[state=closed]:animate-out data-[state=open]:animate-in", className, )} {...props} > {children} - </CollapsibleContent> - </Collapsible> + </CollapsibleContent> );Also applies to: 78-104, 114-126
lib/ai/tools/index.ts (1)
38-50: Expose a safe drain helper for the accumulator to avoid stale/duplicate IDs across requests.Providing
drainFileAccumulator()makes the intended lifecycle explicit, especially in serverless handlers.const todoManager = new TodoManager(initialTodos); const fileAccumulator = new FileAccumulator(); @@ const getSandbox = () => sandbox; const getTodoManager = () => todoManager; const getFileAccumulator = () => fileAccumulator; + const drainFileAccumulator = () => { + const ids = fileAccumulator.getAll(); + fileAccumulator.clear(); + return ids; + }; - return { tools, getSandbox, getTodoManager, getFileAccumulator }; + return { tools, getSandbox, getTodoManager, getFileAccumulator, drainFileAccumulator };If
app/api/chat/route.tscurrently callsgetFileAccumulator().getAll()and thenclear(), consider switching todrainFileAccumulator()once added.Also applies to: 80-83
app/hooks/useChatHandlers.ts (1)
255-261: Type consistency: prefer Id<'messages'> in Convex validator over stringYou cast messageId to Id<"messages"> here, while the Convex mutation (convex/messages.ts Lines 626-725 in context) validates messageId as v.string(). Prefer v.id("messages") on the server and drop the cast here for stronger typing.
Based on learnings
lib/ai/tools/write-file.ts (1)
50-51: Harden against missing accumulator or user IDIf ToolContext.userID or context.fileAccumulator is ever absent, upload/accumulate will throw and leave the user with a sandbox write but no upload. Add a clear guard.
- const saved = await uploadSandboxFileToConvex({ + if (!context.userID) { + return { result: "Wrote file to sandbox, but cannot upload without user session." }; + } + const saved = await uploadSandboxFileToConvex({ sandbox, userId: context.userID, fullPath: file_path, }); - context.fileAccumulator.add(saved.fileId); + context.fileAccumulator?.add(saved.fileId);lib/chat/chat-processor.ts (1)
88-95: Broaden PDF detection fallback (optional)Some uploads may lack accurate mediaType; consider also checking filename extension (.pdf) when mediaType is missing.
- const containsPdfFiles = messagesWithUrls.some((message: any) => - (message.parts || []).some( - (part: any) => - part?.type === "file" && part?.mediaType === "application/pdf", - ), - ); + const containsPdfFiles = messagesWithUrls.some((message: any) => + (message.parts || []).some((part: any) => { + if (part?.type !== "file") return false; + if (part?.mediaType === "application/pdf") return true; + const name = (part?.filename || part?.name || "").toLowerCase(); + return name.endsWith(".pdf"); + }), + );convex/fileStorage.ts (2)
33-54: Avoid full count for limit check (perf)checkFileUploadLimit fetches all user files via countUserFiles; you only need to know if the user has ≥100. Query up to 100/101 and short-circuit.
-export const checkFileUploadLimit = internalQuery({ - args: { userId: v.string() }, - returns: v.null(), - handler: async (ctx, args) => { - const currentFileCount = await ctx.runQuery( - internal.fileStorage.countUserFiles, - { userId: args.userId }, - ); - if (currentFileCount >= 100) { - throw new Error("Upload limit exceeded: Maximum of 100 files allowed per user"); - } - return null; - }, -}); +export const checkFileUploadLimit = internalQuery({ + args: { userId: v.string() }, + returns: v.null(), + handler: async (ctx, args) => { + const sample = await ctx.db + .query("files") + .withIndex("by_user_id", (q) => q.eq("user_id", args.userId)) + .take(100); + if (sample.length >= 100) { + throw new Error("Upload limit exceeded: Maximum of 100 files allowed per user"); + } + return null; + }, +});
60-81: Service-key path skips paid-plan gating
Service flows enforce only upload limits but bypass the paid-entitlement checks applied in the user-authenticated branch. If backend uploads via serviceKey should require a paid plan, add a similar entitlement guard before generating the URL.lib/ai/tools/run-terminal-cmd.ts (3)
164-171: Watcher cleanup is best-effort; consider explicit await when supportedcloseWatcher defers close by 500ms and ignores errors. If SDK exposes an async close/stop, prefer awaiting it once per run to avoid dangling watchers; otherwise keep as is.
Can you confirm whether watchHandle exposes a promise-based close/stop in the current @e2b/code-interpreter version?
130-136: Guard missing userID before attempting uploadsIf context.userID is falsy, uploads will fail server-side. Short-circuit to skip uploads and avoid noisy errors.
const saved = await uploadSandboxFileToConvex({ - sandbox, - userId: context.userID, + sandbox, + userId: context.userID!, fullPath, });Or early-return in flushUploads when !context.userID.
233-239: Confirm downstream consumer for fileUrls in tool resultfileUrls is returned from the tool but I don’t see a consumer in this PR. If unused, consider omitting to reduce payload size; otherwise document the shape.
lib/ai/tools/utils/sandbox-file-uploader.ts (2)
8-23: Set accurate Content-Type based on file extension (optional but helpful)Defaulting to application/octet-stream works but degrades preview. Infer MIME from the filename when possible.
+import { lookup as mimeLookup } from "mime-types"; @@ - const blob = toBlob(data); - const mediaType = DEFAULT_MEDIA_TYPE; + const blob = toBlob(data); + const mediaType = + mimeLookup(fullPath) || DEFAULT_MEDIA_TYPE;Also applies to: 68-76
66-69: Validate response shape; narrow typesAssume upload JSON shape and saved return are stable. If the action evolves, type assertions may mask bugs. Prefer runtime zod validation or at least basic guards.
Would you like a small zod schema to validate the { storageId } and UploadedFileInfo response?
Also applies to: 78-79
app/api/chat/route.ts (1)
249-261: Clear file accumulator after persisting to avoid accidental reusePrevents carry-over on subsequent regenerations in the same process.
for (const message of messages) { await saveMessage({ chatId, userId, message, extraFileIds: message.role === "assistant" ? (newFileIds as unknown as string[]) : undefined, }); } + getFileAccumulator().clear?.();convex/fileActions.ts (3)
400-406: Add a timeout to fetch the storage URL to avoid hanging actions.A stuck fetch can hold action workers. Use AbortController with a reasonable timeout.
- const response = await fetch(fileUrl); + const ac = new AbortController(); + const t = setTimeout(() => ac.abort(), 30_000); + let response; + try { + response = await fetch(fileUrl, { signal: ac.signal }); + } finally { + clearTimeout(t); + }
6-8: Unify LangChain loader imports (community vs core).PDF/CSV come from @langchain/community while JSONLoader comes from langchain/. This mix often breaks with newer LangChain versions. Prefer the community package for consistency.
-import { JSONLoader } from "langchain/document_loaders/fs/json"; +import { JSONLoader } from "@langchain/community/document_loaders/fs/json";Please confirm the repo’s LangChain versions so we can adjust all loaders consistently.
Also applies to: 300-303
391-391: Verify the userId contract (auth subject vs DB Id<'users'>).actingUserId is a string (auth subject). Ensure internal.fileStorage.checkFileUploadLimit/saveFileToDb expect the same identifier, not an Id<'users'>. If they expect a DB id, add a lookup/mapper; if they expect subject, consider renaming to actorSubject for clarity.
Also applies to: 451-451
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
.env.local.example(1 hunks)app/api/chat/route.ts(2 hunks)app/components/ReasoningHandler.tsx(1 hunks)app/hooks/useChatHandlers.ts(1 hunks)components/ai-elements/reasoning.tsx(1 hunks)convex/fileActions.ts(3 hunks)convex/fileStorage.ts(1 hunks)convex/messages.ts(3 hunks)lib/ai/providers.ts(1 hunks)lib/ai/tools/index.ts(4 hunks)lib/ai/tools/run-terminal-cmd.ts(6 hunks)lib/ai/tools/utils/file-accumulator.ts(1 hunks)lib/ai/tools/utils/sandbox-file-uploader.ts(1 hunks)lib/ai/tools/write-file.ts(2 hunks)lib/chat/chat-processor.ts(3 hunks)lib/db/actions.ts(3 hunks)lib/system-prompt.ts(0 hunks)types/agent.ts(2 hunks)
💤 Files with no reviewable changes (1)
- lib/system-prompt.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
lib/ai/tools/write-file.tslib/chat/chat-processor.tstypes/agent.tsconvex/messages.tsconvex/fileStorage.tslib/ai/tools/utils/file-accumulator.tscomponents/ai-elements/reasoning.tsxapp/components/ReasoningHandler.tsxconvex/fileActions.tslib/db/actions.tslib/ai/tools/utils/sandbox-file-uploader.tslib/ai/providers.tsapp/hooks/useChatHandlers.tslib/ai/tools/index.tslib/ai/tools/run-terminal-cmd.tsapp/api/chat/route.ts
convex/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.ts: Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
When a function returns null, include returns: v.null() and return null explicitly
Use internalQuery/internalMutation/internalAction for private functions callable only by other Convex functions; do not expose sensitive logic via public query/mutation/action
Use query/mutation/action only for public API functions
Do not try to register functions via the api or internal objects
Always include argument and return validators for all Convex functions (query/internalQuery/mutation/internalMutation/action/internalAction)
In JS implementations, functions without an explicit return value implicitly return null
Use ctx.runQuery from queries/mutations/actions to call a query
Use ctx.runMutation from mutations/actions to call a mutation
Use ctx.runAction from actions to call an action
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise extract shared helper code
Minimize calls from actions to queries/mutations to avoid race conditions from splitting transactions
Pass FunctionReference values (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do not pass function implementations
When calling a function in the same file via ctx.run*, add an explicit return type annotation at the call site to avoid TS circularity
Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Respect file-based routing for function references: e.g., convex/example.ts export f -> api.example.f; nested paths map to dot-separated namespaces
For paginated queries use paginationOptsValidator in args and .paginate(args.paginationOpts) on a query
v.bigint() is deprecated; use v.int64() for signed 64-bit integers
Use v.record(keys, values) for record-like data; v.map() and v.set() are not supported
For full-text search, use withSearchIndex("ind...
Files:
convex/messages.tsconvex/fileStorage.tsconvex/fileActions.ts
🧠 Learnings (5)
📚 Learning: 2025-09-06T13:31:08.519Z
Learnt from: RostyslavManko
PR: hackerai-tech/hackerai#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/chat/chat-processor.ts
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
PR: hackerai-tech/hackerai#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/fileStorage.tsconvex/fileActions.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
convex/fileStorage.tsconvex/fileActions.tslib/ai/tools/utils/sandbox-file-uploader.ts
📚 Learning: 2025-08-27T12:38:06.662Z
Learnt from: RostyslavManko
PR: hackerai-tech/hackerai#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:
.env.local.example
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
lib/ai/tools/utils/sandbox-file-uploader.ts
🧬 Code graph analysis (12)
lib/ai/tools/write-file.ts (1)
lib/ai/tools/utils/sandbox-file-uploader.ts (1)
uploadSandboxFileToConvex(39-79)
types/agent.ts (1)
lib/ai/tools/utils/file-accumulator.ts (1)
FileAccumulator(3-21)
convex/fileStorage.ts (1)
convex/chats.ts (1)
validateServiceKey(5-9)
lib/ai/tools/utils/file-accumulator.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
app/components/ReasoningHandler.tsx (1)
components/ai-elements/reasoning.tsx (1)
Reasoning(37-66)
convex/fileActions.ts (1)
convex/chats.ts (1)
validateServiceKey(5-9)
lib/db/actions.ts (1)
lib/utils/file-token-utils.ts (1)
extractFileIdsFromParts(21-36)
lib/ai/tools/utils/sandbox-file-uploader.ts (1)
convex/_generated/dataModel.d.ts (1)
Id(48-49)
app/hooks/useChatHandlers.ts (2)
convex/_generated/api.js (2)
api(21-21)api(21-21)convex/messages.ts (2)
saveAssistantMessage(250-310)regenerateWithNewContent(627-726)
lib/ai/tools/index.ts (1)
lib/ai/tools/utils/file-accumulator.ts (1)
FileAccumulator(3-21)
lib/ai/tools/run-terminal-cmd.ts (1)
lib/ai/tools/utils/sandbox-file-uploader.ts (1)
uploadSandboxFileToConvex(39-79)
app/api/chat/route.ts (3)
lib/ai/tools/index.ts (1)
createTools(17-83)convex/messages.ts (1)
saveMessage(49-128)lib/db/actions.ts (1)
saveMessage(46-81)
🪛 dotenv-linter (3.3.0)
.env.local.example
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The CONVEX_SERVICE_ROLE_KEY key should go before the NEXT_PUBLIC_CONVEX_URL key
(UnorderedKey)
🔇 Additional comments (11)
convex/messages.ts (2)
250-310: Client save path looks good; confirm fileIds flow.Auth is derived server-side (good). This path doesn’t accept fileIds, while service‑key saveMessage does. If the new FileAccumulator pipeline attaches files via the backend mutation only, this is fine; otherwise, consider parity here.
Can you confirm that assistant messages with attachments are always persisted via saveMessage (serviceKey) and never via saveAssistantMessage?
627-702: Index shape check for range query on _creationTime.The chained .gt("_creationTime", message._creationTime) inside withIndex("by_chat_id", …) requires _creationTime to be the next field in that index. If by_chat_id doesn’t include _creationTime, add a composite index (chat_id, _creationTime) and keep this range query; otherwise pagination or a secondary query is safer.
Would you confirm the current definition of the by_chat_id index in your schema matches (chat_id, _creationTime)?
lib/ai/providers.ts (1)
13-16: Verify provider model IDs and implement fallbacks
- Confirm
qwen/qwen3-vl-235b-a22b-instructis available (OpenRouter check passed).- OpenAI check returned null
.data(authentication or endpoint issue); confirmgpt-4.1-2025-04-14andgpt-4.1-mini-2025-04-14exist andOPENAI_API_KEYis set.- Implement a fallback map (e.g., default to an OpenAI vision-capable model if
OPENROUTER_API_KEYis missing) to avoid runtime failures.lib/chat/chat-processor.ts (2)
18-37: selectModel invocations correctly pass the PDF flag
Verified that all calls toselectModelinclude thecontainsPdfFilesargument; no further changes needed.
112-114: Mapping Confirmed; Please Verify Pricing/Limits
Thevision-model-for-pdfsentry is present in lib/ai/providers.ts (line 14). Ensure your external pricing and rate-limit configurations align with this model’s usage.app/hooks/useChatHandlers.ts (2)
62-70: Endpoint renames verified—no stale “FromClient” references
No lingering old endpoint names detected in the codebase.
199-208: Align regenerate payload with server-side deletionFor non-temporary chats, the client still includes the last assistant message in
body.messageseven though the server deletes it—if the server uses that array for context, they’ll be out of sync. Confirm inapp/api/chat/route.ts(or wherever regenerate is implemented) that the handler ignores the final assistant message inbody.messages; otherwise, drop it from the payload in both temporary and non-temporary flows.lib/ai/tools/write-file.ts (1)
39-56: Approve code: write→upload→accumulate flow is correct;ToolContextalready includesfileAccumulatoranduserID.convex/fileActions.ts (3)
364-366: Args surface for serviceKey/userId looks good; optional in dev, enforced via handler path.This aligns with our pattern of making serviceKey optional for local DX while enforcing in prod via validation. No changes needed.
Based on learnings
373-387: Service‑role path is correctly gated and requires userId when serviceKey is used.Good separation between service role and end‑user auth.
448-458: Good use of internal mutation from action (no DB access in actions).Pattern is compliant with Convex guidelines.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/hooks/useFileUpload.ts (1)
36-39: Good wrapper to match the util’s expected signature; consider locking the type.Wrapping the mutation is correct. To prevent future drift and make intent explicit, add an explicit function type:
- const generateUploadUrlFn = useCallback(() => generateUploadUrl({}), [ - generateUploadUrl, - ]); + const generateUploadUrlFn: () => Promise<string> = useCallback( + () => generateUploadUrl({}), + [generateUploadUrl], + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/hooks/useFileUpload.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
app/hooks/useFileUpload.ts
🧠 Learnings (5)
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use ctx.storage.getUrl(fileId) to obtain signed URLs; it returns null if the file does not exist
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use the generated api object for public functions and internal object for internal functions from convex/_generated/api.ts
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Always use the new Convex function syntax (query/mutation/action objects with args/returns/handler) when defining Convex functions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Use query/mutation/action only for public API functions
Applied to files:
app/hooks/useFileUpload.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to convex/**/*.ts : Store and handle files as Blob objects; convert to/from Blob when using Convex storage
Applied to files:
app/hooks/useFileUpload.ts
🧬 Code graph analysis (1)
app/hooks/useFileUpload.ts (1)
convex/fileStorage.ts (1)
generateUploadUrl(59-115)
🔇 Additional comments (1)
app/hooks/useFileUpload.ts (1)
180-186: Confirm per-file URL invocation
uploadSingleFileToConvex invokes generateUploadUrlFn on each call, ensuring a fresh, single-use URL per file.
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)
app/components/FilePartRenderer.tsx (1)
163-181: Fix stalerenderedFilePartmemo dependencies.
renderedFilePartcloses overtotalFilePartsand readspart.name,part.filename, andpart.mediaType, but these values aren’t listed in the dependency array. When any of them change (e.g., additional saved files arrive), the memo keeps returning the old element, so image layouts and metadata stay stuck on the previous values. Add every consumed field to the dependency list to keep the preview in sync.- }, [messageId, partIndex, part.url, part.fileId]); + }, [ + messageId, + partIndex, + part.url, + part.fileId, + part.name, + part.filename, + part.mediaType, + totalFileParts, + ]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/api/chat/route.ts(3 hunks)app/components/FilePartRenderer.tsx(5 hunks)app/components/Messages.tsx(4 hunks)app/hooks/useChatHandlers.ts(1 hunks)app/hooks/useFileUpload.ts(2 hunks)lib/ai/tools/get-terminal-files.ts(1 hunks)lib/ai/tools/index.ts(5 hunks)lib/ai/tools/run-terminal-cmd.ts(3 hunks)lib/ai/tools/write-file.ts(2 hunks)lib/utils/message-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/hooks/useChatHandlers.ts
- app/hooks/useFileUpload.ts
- lib/ai/tools/write-file.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
When defining Record types, specify key and value types matching validators (e.g., Record<Id<'users'>, string>)
Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Useas constfor string literals in discriminated unions
Declare arrays with explicit generic type: const arr: Array = [...]
Declare records with explicit generic types: const record: Record<KeyType, ValueType> = {...}
Files:
lib/ai/tools/get-terminal-files.tslib/ai/tools/index.tslib/ai/tools/run-terminal-cmd.tsapp/components/Messages.tsxapp/components/FilePartRenderer.tsxapp/api/chat/route.tslib/utils/message-utils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: RostyslavManko
PR: hackerai-tech/hackerai#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.
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to **/*.{ts,tsx} : Be strict with types for document IDs; prefer Id<'table'> over string in function args and variables
Applied to files:
app/api/chat/route.ts
📚 Learning: 2025-08-29T13:33:09.937Z
Learnt from: CR
PR: hackerai-tech/hackerai#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-29T13:33:09.937Z
Learning: Applies to **/*.{ts,tsx} : Use Id helper type from ./_generated/dataModel to type document IDs (e.g., Id<'users'>) instead of string
Applied to files:
app/api/chat/route.ts
🧬 Code graph analysis (4)
lib/ai/tools/get-terminal-files.ts (2)
types/agent.ts (1)
ToolContext(17-26)lib/ai/tools/utils/sandbox-file-uploader.ts (1)
uploadSandboxFileToConvex(39-79)
lib/ai/tools/index.ts (2)
lib/ai/tools/utils/file-accumulator.ts (1)
FileAccumulator(3-21)lib/ai/tools/get-terminal-files.ts (1)
createGetTerminalFiles(6-68)
app/components/Messages.tsx (3)
types/chat.ts (1)
ChatMessage(68-68)lib/utils/message-utils.ts (1)
extractSavedFilesFromMessage(113-149)app/components/FilePartRenderer.tsx (1)
FilePartRenderer(200-215)
app/api/chat/route.ts (3)
lib/ai/tools/index.ts (1)
createTools(18-85)lib/db/actions.ts (1)
saveMessage(46-81)convex/messages.ts (1)
saveMessage(49-128)
🔇 Additional comments (1)
app/api/chat/route.ts (1)
249-259: Restore Id<'files'> type-safety for saved assistant attachments.
getFileAccumulator().getAll()already returnsArray<Id<"files">>, yet we downcast tostring[]to satisfysaveMessage’s current signature (which then casts back toany). This breaks the compile-time guarantee that we only persist Convex file IDs and reopens the door to mixing arbitrary strings infileIds, despite the Convex mutation expectingv.id("files"). Please carry the Id<'files'> type end-to-end instead of casting.Recommended fix: widen
saveMessageto acceptArray<Id<"files">>and drop the casts here.- const newFileIds = getFileAccumulator().getAll(); + const newFileIds = getFileAccumulator().getAll(); @@ - extraFileIds: - message.role === "assistant" - ? (newFileIds as unknown as string[]) - : undefined, + extraFileIds: + message.role === "assistant" ? newFileIds : undefined,And in
lib/db/actions.ts:+import type { Id } from "@/convex/_generated/dataModel"; @@ - extraFileIds?: string[]; + extraFileIds?: Array<Id<"files">>; @@ - const mergedFileIds = [ - ...fileIds, - ...((extraFileIds || []).filter(Boolean) as string[]), - ]; + const mergedFileIds: Array<Id<"files">> = [ + ...(fileIds as Array<Id<"files">>), + ...((extraFileIds || []).filter(Boolean) as Array<Id<"files">>), + ]; @@ - fileIds: mergedFileIds.length > 0 ? (mergedFileIds as any) : undefined, + fileIds: mergedFileIds.length > 0 ? mergedFileIds : undefined,This keeps us aligned with the Convex schema and the project guideline to use
Id<'table'>for document identifiers. As per coding guidelines
Summary by CodeRabbit
Release Notes
New Features
Improvements
API Changes
Bug Fixes