fix(zip-upload): increased limit for upload #1124
Conversation
SahilKumar000
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
Summary of ChangesHello @SahilKumar000, 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 enhances the zip file upload functionality by increasing the permissible limits for both the total size of a zip archive and the number of files it can contain upon extraction. These changes aim to accommodate larger datasets and improve the overall robustness of the knowledge base upload process, alongside minor code cleanup. 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. WalkthroughUpdated knowledge base upload limits: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 increases the upload limits for zip files, specifically raising the maximum zip file size from 25MB to 35MB and the maximum number of files within a zip from 3000 to 10000. The changes are straightforward, but I've pointed out a couple of places where comments have become outdated due to these value changes. I've also suggested moving a magic number to a top-level constant to improve code maintainability.
![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
♻️ Duplicate comments (1)
server/api/knowledgeBase.ts (1)
1288-1289: Update the outdated comment and consider extracting as a top-level constant.The comment on line 1288 still references 3,000 files, but the limit has been increased to 10,000. Additionally, for better maintainability and consistency with
MAX_ZIP_FILE_SIZE, consider definingmaxFilesLimitas a top-level constant near line 108.Apply this diff:
const KB_STORAGE_ROOT = join(process.cwd(), "storage", "kb_files") const MAX_FILE_SIZE = 100 // 100MB max file size const MAX_ZIP_FILE_SIZE = 35 // 35MB max zip file size +const MAX_FILES_LIMIT = 10000 // 10000 files max per upload requestThen update the usage:
- // Validate file count - allow up to 3000 files for zip extractions - const maxFilesLimit = 10000 + // Validate file count after zip extraction + const maxFilesLimit = MAX_FILES_LIMITAdditionally, verify the performance implications:
This is a 233% increase in the file limit. With 10,000 files being processed:
- Database transaction times may increase significantly
- Memory consumption during batch processing could spike
- Queue processing times will be longer
#!/bin/bash # Check if there are any batch size limits, memory limits, or transaction timeout configurations rg -n -C3 "batch|timeout|memory.*limit|transaction.*timeout" server/api/knowledgeBase.ts server/queue/ server/config/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/api/knowledgeBase.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
server/api/knowledgeBase.ts (2)
server/utils.ts (1)
getErrorMessage(103-106)server/shared/fileUtils.ts (1)
getFileType(10-32)
🔇 Additional comments (2)
server/api/knowledgeBase.ts (2)
1744-1744: Good improvement to structured logging.The change from string concatenation to a structured error object format improves log parsing and analysis capabilities.
1939-1958: Improved readability through reformatting.The extraction of
isSheetFileinto a multi-line expression improves code readability without changing functionality.
| const KB_STORAGE_ROOT = join(process.cwd(), "storage", "kb_files") | ||
| const MAX_FILE_SIZE = 100 // 100MB max file size | ||
| const MAX_ZIP_FILE_SIZE = 25 // 25MB max zip file size | ||
| const MAX_ZIP_FILE_SIZE = 35 // 35MB max zip file size |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the increased zip upload limit.
The MAX_ZIP_FILE_SIZE has been increased from 25MB to 35MB (40% increase). While the comment now correctly reflects this value, ensure that:
- This increase has been tested with actual 35MB zip files
- Server resources (memory, processing time) can handle larger zip extractions
- The change aligns with infrastructure capacity and user requirements
🏁 Script executed:
#!/bin/bash
# Search for any configuration or documentation that might reference the old 25MB limit
rg -n -C3 --type=md --type=yaml --type=json "25.*MB|25MB" Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for "25" in TypeScript/JavaScript files to find old limit references
rg -n --type=ts --type=js "25" | head -50Length of output: 3763
🏁 Script executed:
#!/bin/bash
# Look for test files related to zip or upload functionality
fd -e test.ts -e spec.ts -e test.js -e spec.js | grep -i "zip\|upload\|knowledge"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the actual file and surrounding context for both limit changes
rg -n "MAX_ZIP_FILE_SIZE|maxFilesLimit" server/api/knowledgeBase.ts -A2 -B2Length of output: 1134
🏁 Script executed:
#!/bin/bash
# Search for any README or documentation that might reference these limits
fd -e md -e txt | xargs grep -l "zip\|upload\|file.*size" 2>/dev/null | head -10Length of output: 118
🏁 Script executed:
#!/bin/bash
# Check the documentation files for references to file limits
for file in observability/docs/observability.md server/CHANGELOG.md server/eval/README.md server/integrations/dataSource/README.md; do
echo "=== $file ==="
grep -n "zip\|upload\|file.*size\|3000\|25.*MB" "$file" 2>/dev/null | head -20
doneLength of output: 1842
🏁 Script executed:
#!/bin/bash
# Search for any comments around the maxFilesLimit change for context
rg -n "maxFilesLimit|3000.*file|10000.*file" server/api/knowledgeBase.ts -B5 -A5Length of output: 542
🏁 Script executed:
#!/bin/bash
# Check if there are any related configuration or environment variables
rg -n "MAX_FILE|MAX_ZIP|maxFiles" --type=ts --type=js | grep -v node_modules | head -30Length of output: 2092
Fix outdated comment and verify both limit increases have been tested.
Line 1288's comment is outdated—it claims "allow up to 3000 files" but the code enforces 10000 (line 1289). The MAX_ZIP_FILE_SIZE comment at line 109 is now correct (35MB).
However, both limit increases lack test coverage:
- MAX_ZIP_FILE_SIZE: 25MB → 35MB (40% increase)
- maxFilesLimit: 3000 → 10000 (233% increase)
Update the comment at line 1288 to reflect 10000, and ensure both limit increases have been stress-tested with actual workloads to verify server memory and processing capacity can handle the larger volumes.
🤖 Prompt for AI Agents
In server/api/knowledgeBase.ts around lines 109 and 1288, the MAX_ZIP_FILE_SIZE
comment at line 109 is accurate (35MB) but the inline comment at line 1288 is
outdated (it says "allow up to 3000 files" while the code enforces 10000);
update that comment to state 10000 files, and add/modify automated and/or manual
stress tests to validate both increases (MAX_ZIP_FILE_SIZE 25→35MB and
maxFilesLimit 3000→10000) under realistic workloads to confirm memory and
processing capacity are acceptable, documenting test results.
## [3.18.6](v3.18.5...v3.18.6) (2025-10-17) ### Bug Fixes * **zip-upload:** increased limit for upload ([#1124](#1124)) ([7f2bc16](7f2bc16))
2 participants
Description
Testing
Additional Notes
Summary by CodeRabbit
Improvements
Chores