-
Notifications
You must be signed in to change notification settings - Fork 183
feat: support uploading pdf, docx, txt #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 8068ad5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as spam.
This comment was marked as spam.
templates/types/streaming/nextjs/app/components/ui/upload-pdf-preview.tsx
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/index.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-pdf .ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-config.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/chat-input.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/embed/route.ts (1 hunks)
Additional comments not posted (1)
templates/types/streaming/nextjs/app/api/chat/embed/route.ts (1)
27-28
: Verify the impact of the runtime configuration change.The change from
force-static
toforce-dynamic
in the runtime configuration could have significant implications on the behavior and performance of the application. It is crucial to ensure that this change is thoroughly tested, especially in production environments.
templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- templates/types/streaming/express/src/controllers/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/express/src/controllers/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-input.tsx (5 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/chat-files.tsx (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx (4 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-file.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/index.ts (3 hunks)
- templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (7)
- templates/types/streaming/express/src/controllers/llamaindex-stream.ts
- templates/types/streaming/express/src/controllers/stream-helper.ts
- templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts
- templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx
- templates/types/streaming/nextjs/app/components/ui/chat/index.ts
Additional comments not posted (7)
templates/types/streaming/nextjs/app/components/ui/chat/chat-message/chat-files.tsx (1)
4-13
: Well-implemented component for rendering file previews.The implementation correctly handles the absence of files and uses a map function to render
FileContentPreview
components. Consider destructuringfiles
fromdata
directly in the function parameter for cleaner code.- export function ChatFiles({ data }: { data: ContentFileData }) { + export function ChatFiles({ data: { files } }: { data: ContentFileData }) {templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-file.ts (3)
19-26
: Good implementation of theupload
function with appropriate checks for duplicates.The use of functional updates in
setFiles
is commendable as it ensures state consistency. Consider improving thefileEqual
function by combining the conditions into a single return statement for clarity.- if (a.id === b.id) return true; - if (a.filename === b.filename && a.filesize === b.filesize) return true; - return false; + return a.id === b.id || (a.filename === b.filename && a.filesize === b.filesize);
37-53
: Robust implementation of PDF detail fetching with proper error handling.The async function is well-structured with appropriate error handling. To enhance the TypeScript usage, consider defining a type for the response data to ensure type safety throughout the function.
- const data = await response.json(); + const data: { content: string; embeddings: number[] } = await response.json();
70-85
: Effective management of annotations based on state.The function correctly gathers annotations from the current state. For better readability, consider using a more descriptive variable name than
annotations
to reflect that these are message annotations.- const annotations: MessageAnnotation[] = []; + const messageAnnotations: MessageAnnotation[] = [];templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (2)
22-60
: Well-structured component for rendering file previews based on type.The conditional rendering logic is clear and effectively separates concerns for different file types. Consider using a switch statement instead of multiple if statements for scalability and clarity as more file types are supported in the future.
- if (file.filetype === "csv") { + switch (file.filetype) { + case "csv":
Line range hint
63-102
: Detailed and well-organizedPreviewDialog
component.The component effectively uses a drawer to display detailed file information. Consider adding aria-labels to interactive elements for improved accessibility.
- <Button variant="outline">Close</Button> + <Button variant="outline" aria-label="Close file preview">Close</Button>templates/types/streaming/nextjs/app/components/ui/chat/chat-input.tsx (1)
Line range hint
25-142
: Robust handling of chat input and file uploads with comprehensive state management.The component effectively handles different file types and integrates annotations into chat messages. Consider refactoring the file type handling into a separate function to improve readability and maintainability.
- if (file.type.startsWith("image/")) { - return await handleUploadImageFile(file); - } - if (file.type === "text/csv") { - return await handleUploadCsvFile(file); - } - if (file.type === "application/pdf") { - return await handleUploadPdfFile(file); - } + return await handleFileUploadBasedOnType(file);
templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/components/ui/chat/chat-input.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- templates/types/streaming/express/package.json (1 hunks)
- templates/types/streaming/express/src/controllers/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/express/src/controllers/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (3 hunks)
- templates/types/streaming/nextjs/package.json (3 hunks)
Files skipped from review due to trivial changes (2)
- templates/types/streaming/express/package.json
- templates/types/streaming/nextjs/package.json
Additional comments not posted (9)
templates/types/streaming/express/src/controllers/llamaindex-stream.ts (1)
14-14
: Update to import statement reflects type changes.The import change from
CsvFile
toContentFile
aligns with the updated type definitions. Ensure all references toContentFile
are correctly updated across the project.templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts (1)
14-14
: Updated import statement to match new data types.The update from
CsvFile
toContentFile
is consistent with the changes in data handling across the project. This should be reflected everywhereContentFile
is used.templates/types/streaming/express/src/controllers/stream-helper.ts (1)
116-133
: New type definitions introduced for file handling.The new types
TextEmbedding
,ContentFileType
,ContentFile
, andContentFileData
are well-defined and cover various aspects of file handling. Ensure these are consistently used across the project.templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1)
116-133
: Consistent type definitions for file handling.The introduction of new types such as
TextEmbedding
,ContentFileType
,ContentFile
, andContentFileData
in this part of the project aligns with the changes in the Express app. Ensure these are used consistently.templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (5)
17-18
: Type Definition Update:FileContentPreviewProps
The update to the interface ensures that it now correctly expects a
ContentFile
type, aligning with the overall system's migration fromCsvFile
toContentFile
. This change is crucial for the integration of multiple file types.
22-63
: Conditional Rendering Based on File TypeThe function correctly handles different file types (
csv
and
66-72
: InterfacePreviewDialogProps
Defined ClearlyThe interface provides a clear and structured way to pass necessary properties to the
PreviewDialog
component, enhancing readability and maintainability of the code.
74-86
: Well-ImplementedPreviewDialog
ComponentThe implementation of
PreviewDialog
is clean and efficient, using theDrawer
components effectively. Consider addingaria-labels
for accessibility improvements.+ <Button variant="outline" aria-label="Close Preview">Close</Button>
105-116
: Effective Use of Conditional Rendering inPreviewCard
The
PreviewCard
function is implemented effectively, with good use of conditional rendering and dynamic class names. Consider enhancing the visual design by adjusting the spacing or color scheme.- <div className="p-2 w-60 max-w-60 bg-secondary rounded-lg text-sm relative cursor-pointer"> + <div className="p-4 w-60 max-w-60 bg-secondary rounded-lg text-sm relative cursor-pointer">
templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/api/chat/embed/embeddings.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/express/src/controllers/llamaindex-stream.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
templates/types/streaming/nextjs/app/components/ui/icons/docx.svg
is excluded by!**/*.svg
templates/types/streaming/nextjs/app/components/ui/icons/pdf.svg
is excluded by!**/*.svg
templates/types/streaming/nextjs/app/components/ui/icons/txt.svg
is excluded by!**/*.svg
Files selected for processing (14)
- .changeset/late-weeks-sneeze.md (1 hunks)
- templates/components/ui/html/chat/hooks/use-config.ts (1 hunks)
- templates/types/streaming/express/src/controllers/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/express/src/controllers/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts (2 hunks)
- templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/chat-section.tsx (2 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/chat-files.tsx (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx (4 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-config.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-file.ts (1 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/index.ts (3 hunks)
- templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (3 hunks)
- templates/types/streaming/nextjs/app/page.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- .changeset/late-weeks-sneeze.md
Additional comments not posted (25)
templates/types/streaming/nextjs/app/components/ui/chat/chat-message/chat-files.tsx (1)
4-13
: Well-implemented rendering logic for file previews.The conditional rendering based on
data.files.length
is a good practice. It ensures that the component does not attempt to render an empty list, which enhances performance and user experience.templates/types/streaming/nextjs/app/page.tsx (1)
6-6
: Good adjustment to container dimensions.Changing the main container dimensions to
h-screen w-screen
ensures that the container uses the entire viewport, which is beneficial for a full-screen application layout.templates/components/ui/html/chat/hooks/use-config.ts (1)
10-30
: Refactored configuration handling with improved clarity and flexibility.The refactoring to use a
backend
property simplifies the configuration management. However, consider enhancing the error handling in the fetch operation to provide a better user experience during configuration failures.- .catch((error) => console.error("Error fetching config", error)); + .catch((error) => { + console.error("Error fetching config", error); + setConfig({ ...config, error: "Failed to fetch configuration" }); + });templates/types/streaming/nextjs/app/components/chat-section.tsx (1)
Line range hint
8-20
: Consistent use of configuration and robust error handling in API interactions.The integration of the
backend
property for API calls is consistent with the changes across other components. The added error handling for JSON parsing enhances the robustness of the component.templates/types/streaming/nextjs/app/components/ui/chat/index.ts (6)
10-10
: Enum addition approved.The addition of
DOCUMENT_FILE
toMessageAnnotationType
is consistent with the PR's focus on enhancing support for various document types.
20-23
: New type addition approved.The
TextEmbedding
type is essential for handling embeddings in document files, aligning well with the PR's objectives.
25-25
: Addition of file type enumeration approved.The
ContentFileType
supports the application's expanded capability to handle various document types.
27-33
: New type for document handling approved.The
DocumentFile
type is well-structured to manage document files along with their metadata and embeddings. Consider adding descriptions for each field in the type definition for better clarity.
36-37
: Type addition for handling multiple document files approved.The
DocumentFileData
type effectively supports the handling of multiple document files, enhancing the application's functionality.
73-73
: Union type extension approved.Including
DocumentFileData
inAnnotationData
allows for document files to be used in annotations, aligning with the PR's enhancements.templates/types/streaming/nextjs/app/components/ui/chat/hooks/use-file.ts (1)
8-99
: Comprehensive review ofuseFile
hook.The
useFile
function is well-structured and aligns with the PR's objectives to manage file operations in the chat application. Here are some specific observations:
- Correctness: The function correctly manages state and handles file operations.
- Security: Ensure that
getPdfDetail
properly sanitizes thepdfBase64
input to avoid potential security risks.- Performance: Consider memoizing the
fileEqual
function if it's called frequently to avoid performance issues.templates/types/streaming/express/src/controllers/llamaindex-stream.ts (1)
14-14
: Import change approved.Replacing
CsvFile
withDocumentFile
is consistent with the PR's enhancements to support various document types.templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts (1)
14-14
: Import change approved.Replacing
CsvFile
withDocumentFile
is consistent with the PR's enhancements to support various document types.templates/types/streaming/nextjs/app/components/ui/file-content-preview.tsx (3)
3-6
: Icon additions for document types approved.The addition of icons for DOCX, PDF, TXT, and CSV files enhances the user interface by providing appropriate visual cues for different file types.
25-37
: Function update for file content preview approved.The
FileContentPreview
function is well-structured and effectively renders file content previews. Consider adding accessibility attributes to improve usability for all users.
63-81
: Function update for rendering file preview cards approved.The
PreviewCard
function effectively renders a compact overview of files. Consider adding tooltips to the icons for better user understanding.templates/types/streaming/express/src/controllers/stream-helper.ts (4)
116-119
: New Type Definition: TextEmbeddingThis type definition appears to be well-defined and aligns with the new functionality for handling various content types. The fields
text
andembedding
are essential for text analysis and machine learning applications.
121-121
: New Type Definition: ContentFileTypeThis type definition is a simple enumeration of supported file types. It's clear and concise, which is good for maintainability.
123-130
: New Type Definition: DocumentFileThis type encapsulates the necessary details of a document file, including metadata and optional embeddings. The structure is comprehensive, supporting a variety of file types and use cases.
132-133
: New Type Definition: DocumentFileDataThis type is effectively a container for an array of
DocumentFile
. It's straightforward and matches the pattern used elsewhere in the system for handling collections of similar entities.templates/types/streaming/nextjs/app/api/chat/stream-helper.ts (1)
123-130
: Duplicate Type Definition: DocumentFileThis type definition is identical to the one in the Express directory. Ensure that this duplication serves a purpose, such as microservice independence.
templates/types/streaming/nextjs/app/components/ui/chat/chat-message/index.tsx (4)
8-8
: Update Import: DocumentFileDataThe import of
DocumentFileData
replaces previous data structures, aligning with the new handling of various content types.
19-19
: Import New Component: ChatFilesThis import is necessary for rendering files in chat messages, supporting the new functionality.
44-46
: Function Update: getAnnotationData for DocumentFileDataThis update is crucial for retrieving the correct type of data based on the message annotations. It ensures that the new file types are properly handled.
75-77
: Conditional Rendering: ChatFiles ComponentThis change integrates the new
ChatFiles
component into the message rendering logic, allowing for the display of file-based content. The conditional rendering is correctly implemented.
templates/types/streaming/express/src/controllers/llamaindex-stream.ts
Outdated
Show resolved
Hide resolved
templates/types/streaming/nextjs/app/api/chat/llamaindex-stream.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- templates/components/llamaindex/typescript/streaming/annotations.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/components/llamaindex/typescript/streaming/annotations.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- templates/components/vectordbs/python/none/generate.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/components/vectordbs/python/none/generate.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- templates/types/streaming/fastapi/app/api/routers/models.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- templates/types/streaming/fastapi/app/api/routers/models.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- templates/types/streaming/express/src/controllers/chat.controller.ts (3 hunks)
- templates/types/streaming/fastapi/app/api/routers/chat.py (4 hunks)
- templates/types/streaming/fastapi/app/api/routers/models.py (5 hunks)
- templates/types/streaming/fastapi/app/api/routers/vercel_response.py (2 hunks)
- templates/types/streaming/fastapi/gitignore (1 hunks)
- templates/types/streaming/fastapi/pyproject.toml (1 hunks)
- templates/types/streaming/nextjs/app/api/chat/route.ts (3 hunks)
- templates/types/streaming/nextjs/app/components/ui/document-preview.tsx (3 hunks)
Files skipped from review due to trivial changes (2)
- templates/types/streaming/fastapi/gitignore
- templates/types/streaming/fastapi/pyproject.toml
Files skipped from review as they are similar to previous changes (5)
- templates/types/streaming/express/src/controllers/chat.controller.ts
- templates/types/streaming/fastapi/app/api/routers/chat.py
- templates/types/streaming/fastapi/app/api/routers/models.py
- templates/types/streaming/nextjs/app/api/chat/route.ts
- templates/types/streaming/nextjs/app/components/ui/document-preview.tsx
Additional comments not posted (3)
templates/types/streaming/fastapi/app/api/routers/vercel_response.py (3)
5-10
: Imports look good.The added imports are necessary for the new functionality introduced in the file.
32-41
: Initialization method looks good.The
__init__
method correctly initializes theVercelStreamResponse
class and uses thecontent_generator
method.
43-89
: Content generator method looks good.The
content_generator
method is correctly implemented and handles asynchronous operations properly.Ensure that the
content_generator
method is correctly invoked in the codebase.Verification successful
The
content_generator
method is correctly invoked in the codebase.The method is used within the same file where it is defined, ensuring proper integration and functionality.
templates/types/streaming/fastapi/app/api/routers/vercel_response.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `content_generator` method in the codebase. # Test: Search for the method usage. Expect: Correct invocation of the method. rg --type python -A 5 $'content_generator'Length of output: 72
Script:
#!/bin/bash # Description: Verify the usage of the `content_generator` method in the codebase. # Test: Search for the method usage. Expect: Correct invocation of the method. rg -tpy -A 5 $'content_generator'Length of output: 1203
Summary by CodeRabbit
New Features
Enhancements
documentIds
parameter for filtering documents.Bug Fixes
/api/files/tool-output
to/api/files/output
.Refactor
Chores
llama-index
dependency version inpyproject.toml
.output
directory to.gitignore
.