fix: TypeScript templates do not create a new LlamaCloud index or upload a file to an existing index. #356
Conversation
🦋 Changeset detectedLatest commit: 0366c34 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 |
WalkthroughThe changes focus on the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- templates/components/vectordbs/python/llamacloud/index.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
templates/components/vectordbs/python/llamacloud/index.py (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
🔇 Additional comments (2)
templates/components/vectordbs/python/llamacloud/index.py (2)
67-70: LGTMThe
default_factorycorrectly initializesLlamaCloudConfig.
94-94: LGTMThe
get_client()function correctly initializesLlamaCloudConfigas expected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
templates/components/vectordbs/typescript/llamacloud/generate.ts (1)
35-38: Approved changes with a minor suggestion.The modifications improve compatibility and align better with the
LLamaCloudFileService.addFileToPipelineAPI. The use ofBlobinstead ofFileis a good choice for better cross-environment support.A minor suggestion to improve code clarity:
Consider destructuring
filenamefrompath.basename(filePath)earlier in the function for better readability. Here's a suggested refactor:- const filename = path.basename(filePath); - const blob = new Blob([buffer], { type: "application/octet-stream" }); - await LLamaCloudFileService.addFileToPipeline(projectId, pipelineId, blob, { - private: false, - filename: filename, - }); + const filename = path.basename(filePath); + const blob = new Blob([buffer], { type: "application/octet-stream" }); + await LLamaCloudFileService.addFileToPipeline(projectId, pipelineId, blob, { + private: false, + filename, + });This change uses object property shorthand, which is a common JavaScript convention when the property name matches the variable name.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- templates/components/vectordbs/typescript/llamacloud/generate.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
templates/components/vectordbs/typescript/llamacloud/generate.ts (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
This reverts commit 81b5d0b.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
templates/components/llamaindex/typescript/documents/upload.ts (1)
19-38: Improved error handling, but consider refining the error message.The addition of error handling is a significant improvement to the function's robustness. The try-catch block effectively captures potential errors, and the specific handling of the ReferenceError for the File class is particularly useful.
Consider refining the error message to be more flexible about Node.js versions. Instead of specifying "Node.js 20 or higher", you could say "a Node.js version that supports the File API". This allows for compatibility with any version that includes this feature, not just version 20 and above.
Here's a suggested modification:
- "File class is not supported in the current Node.js version. Please use Node.js 20 or higher.", + "File class is not supported in the current Node.js version. Please use a Node.js version that supports the File API.",This change makes the error message more adaptable to future Node.js releases and avoids potential confusion if the File API becomes available in versions prior to 20.
e2e/shared/streaming_template.spec.ts (1)
30-35: LGTM! Consider extracting the skip condition for better maintainability.The conditional skip for Node.js 18 and LlamaCloud data source is well-implemented and clearly explained. This change improves the test suite by avoiding incompatible test scenarios.
Consider extracting the skip condition into a separate function for better maintainability:
function shouldSkipTest(nodeVersion: string, dataSource: string): boolean { return nodeVersion.startsWith("v18") && dataSource === "--llamacloud"; } // Usage test.skip(shouldSkipTest(process.version, dataSource), "Skipping tests for Node 18 and LlamaCloud data source");This refactoring would make it easier to update or extend the skip conditions in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- e2e/shared/streaming_template.spec.ts (1 hunks)
- templates/components/llamaindex/typescript/documents/upload.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
templates/components/llamaindex/typescript/documents/upload.ts (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
leehuwuj
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
templates/components/vectordbs/typescript/llamacloud/queryFilter.ts (1)
4-4: Improved clarity in comment for public documents definition.The updated comment provides more specific and accurate information about how public documents are created. This change enhances the understanding for developers working with this code, aligning well with the subsequent logic that checks for the absence of the "private" field.
Consider adding a brief explanation of how this distinction between public and private documents affects the filter generation, to further improve the documentation.
templates/types/streaming/fastapi/app/api/routers/chat_config.py (2)
29-32: Approve with suggestion: Consider using a more appropriate HTTP status code.The new error handling for missing
LLAMA_CLOUD_API_KEYis a good addition that enhances the robustness of the API. However, consider using HTTP status code 503 (Service Unavailable) instead of 500 (Internal Server Error) for this scenario. Status code 503 better represents that the service is temporarily unavailable due to a configuration issue.Here's a suggested improvement:
if not os.getenv("LLAMA_CLOUD_API_KEY"): raise HTTPException( - status_code=500, detail="LlamaCloud API KEY is not configured" + status_code=503, detail="LlamaCloud service is unavailable: API key is not configured" )
4-4: Overall: Solid improvement in error handling and API robustness.The changes introduce proper error handling for the LlamaCloud configuration, which aligns with the PR objectives. This enhancement improves the reliability of the API and provides clear feedback when the necessary configuration is missing. The implementation follows FastAPI best practices and integrates well with the existing code structure.
Consider implementing similar error handling for other environment variables used in this file (e.g.,
CONVERSATION_STARTERS,LLAMA_CLOUD_INDEX_NAME,LLAMA_CLOUD_PROJECT_NAME) to ensure consistent behavior across all configuration-dependent features.Also applies to: 29-32
templates/types/streaming/nextjs/app/components/ui/chat/widgets/LlamaCloudSelector.tsx (1)
69-78: Improved error handling for fetch operation.The changes enhance the robustness of the API call by properly handling non-OK responses and providing user feedback. This is a good improvement.
Consider using a more user-friendly error message format. Instead of displaying raw JSON, you could extract specific fields from the error response for a cleaner presentation. For example:
if (!response.ok) { return response.json().then((errorData) => { const errorMessage = errorData.message || 'Unknown error occurred'; window.alert(`Error: ${errorMessage}`); }); }This assumes the error response has a
messagefield. Adjust according to your API's error response structure.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- templates/components/vectordbs/python/llamacloud/generate.py (1 hunks)
- templates/components/vectordbs/python/llamacloud/query_filter.py (1 hunks)
- templates/components/vectordbs/typescript/llamacloud/generate.ts (1 hunks)
- templates/components/vectordbs/typescript/llamacloud/queryFilter.ts (1 hunks)
- templates/types/streaming/fastapi/app/api/routers/chat_config.py (2 hunks)
- templates/types/streaming/nextjs/app/components/ui/chat/widgets/LlamaCloudSelector.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- templates/components/vectordbs/python/llamacloud/query_filter.py
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/components/vectordbs/typescript/llamacloud/generate.ts
🧰 Additional context used
📓 Path-based instructions (4)
templates/components/vectordbs/python/llamacloud/generate.py (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.templates/components/vectordbs/typescript/llamacloud/queryFilter.ts (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.templates/types/streaming/fastapi/app/api/routers/chat_config.py (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.templates/types/streaming/nextjs/app/components/ui/chat/widgets/LlamaCloudSelector.tsx (1)
Pattern
templates/**: For files under thetemplatesfolder, do not report 'Missing Dependencies Detected' errors.
🔇 Additional comments (5)
templates/components/vectordbs/typescript/llamacloud/queryFilter.ts (1)
Line range hint
1-30: LGTM: Function logic is sound and comment update improves clarity.The
generateFiltersfunction correctly handles both public and private documents based on the presence ofdocumentIds. The updated comment improves understanding without affecting the logic. The rest of the file remains unchanged and functions as expected.templates/types/streaming/fastapi/app/api/routers/chat_config.py (1)
4-4: LGTM: Appropriate import for error handling.The addition of
HTTPExceptionimport fromfastapiis correct and necessary for the new error handling mechanism implemented in the/llamacloudroute.templates/components/vectordbs/python/llamacloud/generate.py (3)
3-3: LGTM: Import statements reorganized for better readabilityThe reorganization of import statements improves code readability by grouping standard library imports, third-party imports, and local imports. The addition of
PipelineTypeimport suggests its usage in the code, which enhances type safety.Also applies to: 8-8, 12-15
Line range hint
1-95: LGTM: Overall structure and logic maintainedThe core functionality of the
ensure_indexandgenerate_datasourcefunctions remains intact. The script continues to use environment variables and initialize settings correctly. The main purpose and workflow of the script have been preserved, maintaining its integrity and functionality.
Line range hint
89-92: Verify the removal of custom metadataThe
custom_metadataparameter has been removed from theadd_file_to_pipelinemethod call. This change removes the explicit setting of the document's visibility ("private": "false").Please confirm if this change is intentional and consider its potential impact on document visibility and searchability in the LlamaCloud index.
To check for any related changes or discussions, you can run:
✅ Verification successful
Removal of
custom_metadataVerifiedThe
custom_metadataparameter has been successfully removed from all instances of theadd_file_to_pipelinemethod calls. Document visibility and searchability are now managed directly throughdoc.metadata["private"], ensuring consistent functionality without thecustom_metadataparameter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related changes or discussions about custom metadata rg --type md --type python 'custom_metadata|document visibility|private'Length of output: 105
Script:
#!/bin/bash # Search for related changes or discussions about custom metadata rg --type md --type py 'custom_metadata|document visibility|private'Length of output: 3481
Script:
#!/bin/bash # Find all calls to add_file_to_pipeline and check for custom_metadata parameter rg --type py 'add_file_to_pipeline\s*\(' # Find all instances where 'private' metadata is set without custom_metadata rg --type py 'doc.metadata\s*\[\s*["\']private["\']\s*\]'Length of output: 448
Script:
#!/bin/bash # Find all calls to add_file_to_pipeline and check for custom_metadata parameter rg --type py 'add_file_to_pipeline\s*\(' # Find all instances where "private" metadata is set without custom_metadata rg --type py "doc.metadata\s*\[\s*[\"']private[\"']\s*\]"Length of output: 704
Script:
#!/bin/bash # Find all calls to add_file_to_pipeline that include the custom_metadata parameter rg --type py 'add_file_to_pipeline\s*\(.*custom_metadata\s*='Length of output: 63
Summary by CodeRabbit
New Features
LlamaCloudConfig, enhancing the handling of environment variables.uploadDocumentfunction for clearer compatibility feedback.LlamaCloudSelectorcomponent for improved user feedback during fetch operations.Bug Fixes
from_envmethod, ensuring a more straightforward configuration setup.Refactor
IndexConfigandget_clientfunctions to utilize the new initialization method forLlamaCloudConfig.loadAndIndexfunction with improved error handling for file uploads.generate.pyfile for clarity.Chores
--llamacloudoption in the data sources.