fix: XYNE-152 attachment upload removal when chat is kb based #1069
Conversation
Ravishekhar7870
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
Summary of ChangesHello @Ravishekhar7870, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a fix to prevent attachment uploads in knowledge base (KB) based chat interfaces. By introducing a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an optional isKnowledgeBaseChat prop to ChatBox and DocumentChat, threads it through KnowledgeManagement, and conditionally disables/hides attachment UI and short-circuits upload/process/send flows when knowledge-base mode is active. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant KM as KnowledgeManagement
participant DC as DocumentChat
participant CB as ChatBox
participant API as Backend/API
User->>KM: Open Knowledge Management page
KM->>DC: Render DocumentChat (isKnowledgeBaseChat=true)
DC->>CB: Render ChatBox (isKnowledgeBaseChat=true)
rect rgba(230,246,255,0.6)
note right of CB: Attachment UI hidden/disabled in KB mode
User->>CB: Attempt attach files / click upload
CB->>CB: uploadFiles() -> early return
CB->>CB: processFiles() -> early return
end
User->>CB: Send message (text)
CB->>API: sendMessage(payload + modelConfig, attachments skipped)
API-->>CB: ack
CB->>User: clear input, reset selected files and previews
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request disables the file attachment feature for knowledge-base chats by introducing an isKnowledgeBaseChat prop. The implementation is mostly correct, but I've found a critical syntax error in the JSX that would prevent the application from compiling. I've also included a few suggestions to improve code style and clarity regarding prop definitions and destructuring. Please address the critical issue to ensure the UI renders correctly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (3)
frontend/src/components/ChatBox.tsx (3)
832-911: AddisKnowledgeBaseChatto the dependency array.The
uploadFilescallback usesisKnowledgeBaseChat(line 835) but doesn't include it in the dependency array (line 910). This violates React hooks rules and could lead to stale closure bugs where the callback doesn't see updated prop values.Apply this diff to fix the dependency array:
}, - [toast], + [toast, isKnowledgeBaseChat], )
918-979: AddisKnowledgeBaseChatto the dependency array.The
processFilescallback usesisKnowledgeBaseChat(line 920) but doesn't include it in the dependency array (line 978). This violates React hooks rules and could lead to stale closure bugs.Apply this diff to fix the dependency array:
}, - [selectedFiles.length, uploadFiles, toast], + [selectedFiles.length, uploadFiles, toast, isKnowledgeBaseChat], )
2515-2531: Misleading success feedback when pasting files in knowledge base mode.When
isKnowledgeBaseChatis true, the paste handler shows a success toast "File pasted...has been added to your message" (lines 2523-2527) even thoughprocessFilessilently rejects the file (guard at line 920). This misleads users into thinking their file was attached when it wasn't.Apply this diff to provide accurate feedback:
if (isValid.length > 0) { + if (isKnowledgeBaseChat) { + toast.info({ + title: "Attachments not available", + description: "File attachments are disabled in knowledge base chat mode.", + }) + return + } // Process the pasted file processFiles([file])
🧹 Nitpick comments (1)
frontend/src/components/ChatBox.tsx (1)
1006-1023: Provide user feedback when files are dropped in knowledge base mode.When
isKnowledgeBaseChatis true and users drag-and-drop files,processFilessilently rejects them (guard at line 920) without any user feedback. Users should be informed why their action had no effect.Apply this diff to add informative feedback:
const handleDrop = useCallback( (e: React.DragEvent) => { e.preventDefault() const files = Array.from(e.dataTransfer.files) if (files.length > 0) { + if (isKnowledgeBaseChat) { + toast.info({ + title: "Attachments not available", + description: "File attachments are disabled in knowledge base chat mode.", + }) + return + } // Check attachment limit before processing if (selectedFiles.length >= MAX_ATTACHMENTS) { toast.error({ title: "Attachment limit reached", description: `You can only attach up to ${MAX_ATTACHMENTS} files at a time.`, }) return } processFiles(files) } }, - [processFiles, selectedFiles.length], + [processFiles, selectedFiles.length, toast, isKnowledgeBaseChat], )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/ChatBox.tsx(9 hunks)frontend/src/components/DocumentChat.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/DocumentChat.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/ChatBox.tsx (4)
frontend/src/types.ts (1)
ToolsListItem(58-61)server/shared/types.ts (1)
AttachmentMetadata(693-693)frontend/src/utils/fileUtils.ts (1)
cleanupPreviewUrls(42-48)frontend/src/components/ui/smart-tooltip.tsx (1)
SmartTooltip(10-97)
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 (2)
frontend/src/components/ChatBox.tsx (2)
832-909: KB mode still allows attachments via paste/drag/drop/programmatic input — block all intake paths.Hiding the button isn’t enough. Users can still add files by drag‑and‑drop or paste. Also the hidden input can be triggered programmatically. Hard‑disable these in KB mode.
Apply these diffs:
@@ - const uploadFiles = useCallback( + const uploadFiles = useCallback( async (files: SelectedFile[]) => { if (files.length === 0) return [] + if (isKnowledgeBaseChat) { + toast.warning({ + title: "Attachments disabled", + description: "Attachments are not allowed in Knowledge Base chats.", + }) + return [] + } @@ - const processFiles = useCallback( + const processFiles = useCallback( (files: FileList | File[]) => { + if (isKnowledgeBaseChat) { + toast.warning({ + title: "Attachments disabled", + description: "Attachments are not allowed in Knowledge Base chats.", + }) + return + } @@ - const handleDrop = useCallback( + const handleDrop = useCallback( (e: React.DragEvent) => { e.preventDefault() + if (isKnowledgeBaseChat) { + return + } const files = Array.from(e.dataTransfer.files) @@ - > - <div className="relative flex items-center"> + > + <div className="relative flex items-center"> @@ - onDragOver={handleDragOver} - onDragLeave={handleDragLeave} - onDrop={handleDrop} + onDragOver={isKnowledgeBaseChat ? undefined : handleDragOver} + onDragLeave={isKnowledgeBaseChat ? undefined : handleDragLeave} + onDrop={isKnowledgeBaseChat ? undefined : handleDrop} @@ - onPaste={(e: React.ClipboardEvent<HTMLDivElement>) => { + onPaste={(e: React.ClipboardEvent<HTMLDivElement>) => { e.preventDefault() const items = e.clipboardData?.items - if (items) { - // Handle file paste (all supported file types) - for (let i = 0; i < items.length; i++) { - const item = items[i] - if (item.kind === "file") { - // Check attachment limit before processing - if (selectedFiles.length >= MAX_ATTACHMENTS) { - toast.error({ - title: "Attachment limit reached", - description: `You can only attach up to ${MAX_ATTACHMENTS} files at a time.`, - }) - return - } - - const file = item.getAsFile() - if (file) { - // Check if the file type is supported - const isValid = validateAndDeduplicateFiles( - [file], - toast, - ) - if (isValid.length > 0) { - // Process the pasted file - processFiles([file]) - const fileType = getFileType({ - type: file.type, - name: file.name, - }) - - toast.success({ - title: "File pasted", - description: `${fileType} has been added to your message.`, - }) - return // Exit early since we handled the file - } else { - return - } - } - } - } - } + if (items) { + const hasFile = Array.from(items).some((it) => it.kind === "file") + if (hasFile && isKnowledgeBaseChat) { + toast.warning({ + title: "Attachments disabled", + description: "Pasting files is disabled in Knowledge Base chats.", + }) + // Fall through to text paste handling below + } else if (hasFile) { + // Handle file paste (all supported file types) + for (let i = 0; i < items.length; i++) { + const item = items[i] + if (item.kind === "file") { + if (selectedFiles.length >= MAX_ATTACHMENTS) { + toast.error({ + title: "Attachment limit reached", + description: `You can only attach up to ${MAX_ATTACHMENTS} files at a time.`, + }) + return + } + const file = item.getAsFile() + if (file) { + const isValid = validateAndDeduplicateFiles([file], toast) + if (isValid.length > 0) { + processFiles([file]) + const fileType = getFileType({ type: file.type, name: file.name }) + toast.success({ + title: "File pasted", + description: `${fileType} has been added to your message.`, + }) + return // Exit early since we handled the file + } else { + return + } + } + } + } + } + } @@ - <input + <input ref={fileInputRef} type="file" multiple className="hidden" - onChange={handleFileChange} + onChange={isKnowledgeBaseChat ? undefined : handleFileChange}Also applies to: 916-975, 1003-1019, 2470-2473, 2486-2634, 3959-3968
2807-2930: Hide attachments preview in KB mode.If attachments are disabled, don’t render the preview tray even if state is somehow populated.
Apply this diff:
- {/* File Attachments Preview */} - {selectedFiles.length > 0 && ( + {/* File Attachments Preview */} + {!isKnowledgeBaseChat && selectedFiles.length > 0 && ( <div className="px-4 py-2 border-t border-gray-100 dark:border-gray-600">
♻️ Duplicate comments (1)
frontend/src/components/ChatBox.tsx (1)
213-214: Prop addition + sane default look good.isKnowledgeBaseChat is optional on the type and defaulted to false in destructuring. Good alignment with prior feedback.
Also applies to: 381-382
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ChatBox.tsx(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/ChatBox.tsx (4)
frontend/src/types.ts (1)
ToolsListItem(58-61)server/shared/types.ts (1)
AttachmentMetadata(693-693)frontend/src/utils/fileUtils.ts (1)
cleanupPreviewUrls(42-48)frontend/src/components/ui/smart-tooltip.tsx (1)
SmartTooltip(10-97)
🔇 Additional comments (1)
frontend/src/components/ChatBox.tsx (1)
2932-2952: Hiding the attach button in KB mode is correct.UI correctly prevents manual selection. With the hard blocks above, behavior matches the PR goal.
## [3.18.5](v3.18.4...v3.18.5) (2025-10-17) ### Bug Fixes * XYNE-152 attachment upload removal when chat is kb based ([#1069](#1069)) ([3c84691](3c84691))
2 participants
Description
removed attachement upload if the chat is kb based
Testing
tested locally
Summary by CodeRabbit
New Features
Chores