Rag-phase 1 #1854
Draft
Rag-phase 1 #1854
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds a complete RAG indexing system (queue, task, processing, embeddings, vector store) and integrates background triggers across multiple API routes. Refactors chat message types to UIMessage and introduces many new UI components (chat, code, citations, loaders). Expands Prisma schema for RAG tracking and updates build/config and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant API as API Route (e.g., link/doc POST)
participant Trigger as triggerDataroomIndexing
participant Queue as RAGQueueManager
participant Task as Trigger.dev ragIndexingTask
participant Proc as DocumentProcessor
participant Chunk as EnhancedChunker
participant Embed as EmbeddingGenerator
participant Vec as QdrantVectorStore
participant DB as Prisma
User->>API: Create/Update/View action
API->>Trigger: waitUntil(triggerDataroomIndexing(dataroomId, teamId, userId))
Trigger->>Queue: addToQueue(dataroomId, request)
Trigger->>Queue: tryStartWorker(dataroomId)
alt Worker started
Trigger->>Task: tasks.trigger("rag-indexing", payload)
else Already running
Trigger-->>API: "queued"
end
Note over Task: Acquire lock, loop until queue empty
Task->>Queue: getNextFromQueue(dataroomId)
Task->>DB: Read RAG status, mark IN_PROGRESS
Task->>Proc: processDocuments(docs, dataroomId, teamId)
Proc->>Chunk: createEnhancedChunks(markdown,…)
Chunk-->>Proc: EnhancedChunk[]
Proc-->>Task: Processing results
Task->>Embed: generateEmbeddings(chunks)
Embed-->>Task: embeddings, token usage
Task->>Vec: upsertPoints(collection, points)
Vec-->>Task: success
Task->>DB: Mark docs COMPLETED, update RAG stats
Task->>Queue: hasPendingRequests? loop/finish
Task->>Queue: releaseLock(dataroomId)
Task-->>API: Background completion (no effect on response)
sequenceDiagram
autonumber
actor Admin
participant API as Delete APIs
participant VM as vectorManager
participant Q as QdrantVectorStore
participant DB as Prisma
Admin->>API: Delete document/folder/dataroom
API->>VM: deleteDocumentVectors / deleteMultipleDocumentVectors / deleteDataroomVectors
VM->>Q: deletePointsByFilter / deleteCollection
Q-->>VM: result
VM-->>API: success/failure
API->>DB: Delete records
API-->>Admin: 204/200 response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes ✨ Finishing touches🧪 Generate unit tests
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 |
|
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 57
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
pages/api/assistants/chat.ts (2)
74-89: Apply plan-specific rate limits (paid/pro paths currently unused).ratelimit.paid and ratelimit.pro are defined but never used. Route all authenticated users through plan-aware buckets.
- if (input.userId && input.plan !== "pro") { - const { success, limit, reset, remaining } = await ratelimit.free.limit( - `ratelimit_${input.userId}`, - ); + if (input.userId) { + const bucket = + input.plan === "pro" + ? ratelimit.pro + : input.plan === "paid" + ? ratelimit.paid + : ratelimit.free; + const { success, limit, reset, remaining } = await bucket.limit( + `ratelimit_${input.userId}`, + );
100-104: Guard against missing Assistant IDs.Non-null assertion can crash at runtime. Fail fast with a clear 500.
- const assistantId = input.isPublic - ? (process.env.OAI_PUBLIC_ASSISTANT_ID as string) - : (process.env.OAI_ASSISTANT_ID as string); + const assistantId = input.isPublic + ? process.env.OAI_PUBLIC_ASSISTANT_ID + : process.env.OAI_ASSISTANT_ID; + if (!assistantId) { + return new Response("Assistant is not configured.", { status: 500 }); + }lib/utils.ts (5)
352-361: Fix filter: metadata.initialMessage is misspelled and type-unsafeCurrently checks
intitialMessage === 'True'(typo) and only string. Handle both spellings and boolean.- const filteredMessages = threadMessages.filter((threadMessage) => { + const filteredMessages = threadMessages.filter((threadMessage) => { if ( typeof threadMessage.metadata === "object" && threadMessage.metadata !== null ) { - // Safely typecast metadata to an object with the expected structure - const metadata = threadMessage.metadata as { intitialMessage?: string }; - return metadata.intitialMessage !== "True"; + const md = threadMessage.metadata as { + initialMessage?: string | boolean; + intitialMessage?: string | boolean; // legacy misspelling + }; + const flag = md.initialMessage ?? md.intitialMessage; + return String(flag).toLowerCase() !== "true"; } return true; // Include messages where metadata is not an object or is null });
373-386: Aggregate all text content; build a single text partOnly the first text item is used; others are dropped. Join all text segments.
- // Assuming content is an array and you want to convert it into a string or JSX element - const messageContent = content.map((item) => { - if (item.type === "text") { - return item.text.value; - } else { - return ""; - } - }); + const text = content + .filter((item) => item.type === "text" && (item as any).text?.value) + .map((item: any) => item.text.value) + .join("\n"); return { id, - role: role === "assistant" ? "assistant" : "user", - parts: [{ type: "text", text: messageContent[0] || "" }], + role: role === "assistant" ? "assistant" : "user", + parts: [{ type: "text", text }], };
546-567: AES key derivation is incorrect; derive a 32-byte key (Buffer), not a base64 string substringUsing
digest('base64').substring(0, 32)yields 24 bytes of UTF-8 text, not 32 raw bytes. This weakens encryption and can fail on some runtimes.export async function generateEncrpytedPassword( password: string, ): Promise<string> { // If the password is empty, return an empty string if (!password) return ""; // If the password is already encrypted, return it const textParts: string[] = password.split(":"); if (textParts.length === 2) { return password; } // Otherwise, encrypt the password - const encryptedKey: string = crypto - .createHash("sha256") - .update(String(process.env.NEXT_PRIVATE_DOCUMENT_PASSWORD_KEY)) - .digest("base64") - .substring(0, 32); - const IV: Buffer = crypto.randomBytes(16); - const cipher = crypto.createCipheriv("aes-256-ctr", encryptedKey, IV); + const secret = String(process.env.NEXT_PRIVATE_DOCUMENT_PASSWORD_KEY || ""); + if (!secret) throw new Error("Missing NEXT_PRIVATE_DOCUMENT_PASSWORD_KEY"); + const key: Buffer = crypto.createHash("sha256").update(secret, "utf8").digest(); // 32 bytes + const IV: Buffer = crypto.randomBytes(16); + const cipher = crypto.createCipheriv("aes-256-ctr", key, IV); let encryptedText: string = cipher.update(password, "utf8", "hex"); encryptedText += cipher.final("hex"); return IV.toString("hex") + ":" + encryptedText; }
569-586: Decryption must use the same 32-byte derived key type as encryptionMirror the fix above.
export function decryptEncrpytedPassword(password: string): string { if (!password) return ""; - const encryptedKey: string = crypto - .createHash("sha256") - .update(String(process.env.NEXT_PRIVATE_DOCUMENT_PASSWORD_KEY)) - .digest("base64") - .substring(0, 32); + const secret = String(process.env.NEXT_PRIVATE_DOCUMENT_PASSWORD_KEY || ""); + if (!secret) return ""; + const key: Buffer = crypto.createHash("sha256").update(secret, "utf8").digest(); const textParts: string[] = password.split(":"); if (!textParts || textParts.length !== 2) { return password; } const IV: Buffer = Buffer.from(textParts[0], "hex"); const encryptedText: string = textParts[1]; - const decipher = crypto.createDecipheriv("aes-256-ctr", encryptedKey, IV); + const decipher = crypto.createDecipheriv("aes-256-ctr", key, IV); let decrypted: string = decipher.update(encryptedText, "hex", "utf8"); decrypted += decipher.final("utf8"); return decrypted; }
350-361: Replace all legacyintitialMessageoccurrences withinitialMessageUpdate the setter in pages/api/assistants/threads/index.ts (line 53) and the filter in lib/utils.ts (lines 350–361) to use the correctly spelled field to prevent metadata mismatches.
pages/api/teams/[teamId]/datarooms/[id]/index.ts (1)
204-206: Allow header and comment are outdated; include DELETE.Clients will get incorrect Allow metadata.
- } else { - // We only allow GET, and PATCH requests - res.setHeader("Allow", ["GET", "PATCH"]); + } else { + // We only allow GET, PATCH and DELETE requests + res.setHeader("Allow", ["GET", "PATCH", "DELETE"]);pages/api/teams/[teamId]/datarooms/[id]/duplicate.ts (2)
210-232: Guard brand creation to avoid creating empty/invalid brand rows.If
dataroomContents.brandis null/undefined this will still attempt a nestedcreate, which can violate non-null constraints or create empty rows.Apply this diff:
folders: { create: [], }, - brand: { - create: { - banner: dataroomContents.brand?.banner, - logo: dataroomContents.brand?.logo, - accentColor: dataroomContents.brand?.accentColor, - brandColor: dataroomContents.brand?.brandColor, - }, - }, + ...(dataroomContents.brand && { + brand: { + create: { + banner: dataroomContents.brand.banner ?? null, + logo: dataroomContents.brand.logo ?? null, + accentColor: dataroomContents.brand.accentColor ?? null, + brandColor: dataroomContents.brand.brandColor ?? null, + }, + }, + }),
111-121: Surface failures from Promise.allSettled to avoid silent partial duplicates.Silent rejections can leave the dataroom half‑copied while returning 201. At minimum, aggregate and log failures; or fail fast if that’s the desired contract.
Apply these diffs:
- await Promise.allSettled( - folder.documents.map((doc) => - prisma.dataroomDocument.create({ - data: { - documentId: doc.documentId, - dataroomId: dataroomId, - folderId: newFolder.id, - }, - }), - ), - ); + { + const results = await Promise.allSettled( + folder.documents.map((doc) => + prisma.dataroomDocument.create({ + data: { + documentId: doc.documentId, + dataroomId: dataroomId, + folderId: newFolder.id, + }, + }), + ), + ); + const failures = results.filter((r) => r.status === "rejected"); + if (failures.length) { + console.error( + `Failed to duplicate ${failures.length} document(s) into folder ${newFolder.id}`, + ); + } + }- await Promise.allSettled( - folder.childFolders.map((childFolder) => - duplicateFolders(dataroomId, childFolder, newFolder.id), - ), - ); + { + const results = await Promise.allSettled( + folder.childFolders.map((childFolder) => + duplicateFolders(dataroomId, childFolder, newFolder.id), + ), + ); + const failures = results.filter((r) => r.status === "rejected"); + if (failures.length) { + console.error( + `Failed to duplicate ${failures.length} child folder(s) under ${newFolder.id}`, + ); + } + }Also applies to: 124-128
pages/api/teams/[teamId]/datarooms/[id]/folders/manage/[folderId]/index.ts
Outdated
Show resolved
Hide resolved
pages/api/teams/[teamId]/folders/manage/[folderId]/add-to-dataroom.ts
Outdated
Show resolved
Hide resolved
Comment on lines
+64
to
+65
| const [hasAutoClosedRef, setHasAutoClosedRef] = useState(false); | ||
| const [startTime, setStartTime] = useState<number | null>(null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
State variable name doesn't match its boolean type
The state variable hasAutoClosedRef is declared as a boolean state but the "Ref" suffix suggests it should be a ref. This naming inconsistency is confusing.
-const [hasAutoClosedRef, setHasAutoClosedRef] = useState(false);
+const [hasAutoClosed, setHasAutoClosed] = useState(false);And update all usages:
-if (defaultOpen && !isStreaming && isOpen && !hasAutoClosedRef) {
+if (defaultOpen && !isStreaming && isOpen && !hasAutoClosed) {
// Add a small delay before closing to allow user to see the content
const timer = setTimeout(() => {
setIsOpen(false);
- setHasAutoClosedRef(true);
+ setHasAutoClosed(true);
}, AUTO_CLOSE_DELAY);🤖 Prompt for AI Agents
In components/reasoning.tsx around lines 64-65, the boolean state variable is
named hasAutoClosedRef which misleadingly implies a ref; rename it to
hasAutoClosed (and the setter to setHasAutoClosed) and update every usage and
import/exports accordingly throughout the file (and any other files referencing
it) so the identifier reflects a boolean state rather than a ref.
Comment on lines
19
to
32
| export interface DocumentChunk { | ||
| id: string; | ||
| content: string; | ||
| metadata: { | ||
| chunkIndex: number; | ||
| documentId: string; | ||
| documentName: string; | ||
| dataroomId: string; | ||
| teamId: string; | ||
| contentType: string; | ||
| pageNumbers?: number[]; | ||
| }; | ||
| chunkHash: string; | ||
| } |
There was a problem hiding this comment.
Type mismatch with downstream usage (pageRanges/tokenCount missing).
DocumentChunk.metadata declares pageNumbers, but code emits pageRanges and later reads tokenCount/sectionHeader in rag-indexing. This will cause TS errors or undefineds.
Update the interface to reflect actual payload:
export interface DocumentChunk {
id: string;
content: string;
metadata: {
chunkIndex: number;
documentId: string;
documentName: string;
dataroomId: string;
teamId: string;
contentType: string;
- pageNumbers?: number[];
+ pageRanges?: string[];
+ tokenCount?: number;
+ sectionHeader?: string;
};
chunkHash: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export interface DocumentChunk { | |
| id: string; | |
| content: string; | |
| metadata: { | |
| chunkIndex: number; | |
| documentId: string; | |
| documentName: string; | |
| dataroomId: string; | |
| teamId: string; | |
| contentType: string; | |
| pageNumbers?: number[]; | |
| }; | |
| chunkHash: string; | |
| } | |
| export interface DocumentChunk { | |
| id: string; | |
| content: string; | |
| metadata: { | |
| chunkIndex: number; | |
| documentId: string; | |
| documentName: string; | |
| dataroomId: string; | |
| teamId: string; | |
| contentType: string; | |
| pageRanges?: string[]; | |
| tokenCount?: number; | |
| sectionHeader?: string; | |
| }; | |
| chunkHash: string; | |
| } |
🤖 Prompt for AI Agents
In lib/rag/document-processor.ts around lines 19 to 32, the
DocumentChunk.metadata currently only declares pageNumbers but downstream code
emits/reads pageRanges and tokenCount/sectionHeader, causing type mismatches and
possible undefined values; update the interface to include pageRanges?:
number[]; tokenCount?: number; sectionHeader?: string; (you may keep
pageNumbers?: number[] for compatibility) and ensure their types match how
rag-indexing expects them so consumers no longer get TS errors or undefined
fields.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Database
Chores