feat: implement file upload size configuration and progress tracking #1108
Conversation
|
Warning Rate limit exceeded@Blackjack200 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis PR makes file upload size configurable via an injected ConfigPresenter, adds a renderer settings section to view/change the max file size, shows real-time file processing progress in KnowledgeFileItem via a new FILE_PROGRESS event, and adds translation keys for the new UI across multiple locales. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as UploadFileSettingsSection
participant Config as ConfigPresenter
participant FilePresenter as FilePresenter
rect rgb(200,220,240)
Note over User,UI: Adjust max file size (settings)
User->>UI: Click +/- or edit value
UI->>Config: setSetting("maxFileSize", valueInBytes)
Config->>Config: Persist setting
end
rect rgb(220,240,200)
Note over FilePresenter,Config: Runtime read
FilePresenter->>Config: getSetting("maxFileSize")
Config-->>FilePresenter: configured limit (bytes)
end
sequenceDiagram
participant App as App / Uploader
participant IPC as IPC Bus
participant Item as KnowledgeFileItem
participant UIState as Progress State
rect rgb(240,220,200)
Note over App,IPC: File processing emits progress
App->>IPC: emit FILE_PROGRESS(payload)
IPC->>Item: FILE_PROGRESS listener receives payload
Item->>UIState: update reactive progress for fileId
Item->>Item: compute progressPercent and counts
Item->>Item: render spinner + tooltip with % & counts
end
rect rgb(240,240,200)
Note over Item,IPC: Cleanup
Item->>Item: onBeforeUnmount()
Item->>IPC: remove FILE_PROGRESS listener
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/settings/components/KnowledgeFileItem.vue (1)
200-217: Consider using a more specific IPC listener cleanup approach.The
removeAllListenerscall on Line 216 removes all listeners for theFILE_PROGRESSevent, which could inadvertently affect other instances of this component or other components listening to the same event.Consider storing a reference to the specific listener and removing only that one:
+const progressListener = ( + _: any, + data: { fileId: string; completed: number; error: number; total: number } +) => { + if (props.file.id === data.fileId) { + progress.value = { + completed: data.completed, + error: data.error, + total: data.total + } + } +} + onMounted(async () => { - window.electron.ipcRenderer.on( - RAG_EVENTS.FILE_PROGRESS, - (_, data: { fileId: string; completed: number; error: number; total: number }) => { - if (props.file.id === data.fileId) { - progress.value = { - completed: data.completed, - error: data.error, - total: data.total - } - } - } - ) + window.electron.ipcRenderer.on(RAG_EVENTS.FILE_PROGRESS, progressListener) }) onBeforeUnmount(() => { - window.electron.ipcRenderer.removeAllListeners(RAG_EVENTS.FILE_PROGRESS) + window.electron.ipcRenderer.off(RAG_EVENTS.FILE_PROGRESS, progressListener) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/main/presenter/filePresenter/FilePresenter.ts(2 hunks)src/main/presenter/index.ts(1 hunks)src/renderer/settings/components/CommonSettings.vue(2 hunks)src/renderer/settings/components/KnowledgeFileItem.vue(4 hunks)src/renderer/settings/components/common/UploadFileSettingsSection.vue(1 hunks)src/renderer/src/events.ts(1 hunks)src/renderer/src/i18n/en-US/settings.json(1 hunks)src/renderer/src/i18n/fa-IR/settings.json(1 hunks)src/renderer/src/i18n/fr-FR/settings.json(1 hunks)src/renderer/src/i18n/ja-JP/settings.json(1 hunks)src/renderer/src/i18n/ko-KR/settings.json(1 hunks)src/renderer/src/i18n/pt-BR/settings.json(1 hunks)src/renderer/src/i18n/ru-RU/settings.json(1 hunks)src/renderer/src/i18n/zh-CN/settings.json(1 hunks)src/renderer/src/i18n/zh-HK/settings.json(1 hunks)src/renderer/src/i18n/zh-TW/settings.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/presenter/filePresenter/FilePresenter.ts (3)
src/shared/types/presenters/legacy.presenters.d.ts (1)
IConfigPresenter(402-607)src/main/presenter/filePresenter/FileValidationService.ts (1)
IFileValidationService(14-18)test/mocks/electron.ts (1)
app(2-10)
src/main/presenter/index.ts (1)
src/main/presenter/filePresenter/FilePresenter.ts (1)
FilePresenter(22-312)
🔇 Additional comments (17)
src/renderer/src/i18n/ru-RU/settings.json (1)
62-64: LGTM! Clean i18n extension for file size settings.The new translation keys
fileMaxSizeandfileMaxSizeHintare properly added with appropriate Russian translations. The trailing comma on line 62 correctly prepares for the new entries.src/renderer/src/i18n/en-US/settings.json (1)
62-64: LGTM! Base English translations added correctly.The English translation keys are clear and descriptive:
fileMaxSize: "File Maximum Size"fileMaxSizeHint: "Limits the maximum size of a single uploaded file"These serve as the reference for other locales and align well with the feature objective.
src/renderer/src/i18n/zh-TW/settings.json (1)
62-64: LGTM! Traditional Chinese translations properly added.The Traditional Chinese translations for the file size settings are consistent with the multilingual expansion of this feature across all locale files.
src/renderer/src/i18n/zh-CN/settings.json (1)
62-64: LGTM! Simplified Chinese translations added correctly.The Simplified Chinese translations ("文件最大大小" and "限制单个文件的最大上传大小") are appropriate and consistent with the i18n expansion across all locales.
src/renderer/src/events.ts (1)
167-168: LGTM! New progress event follows existing patterns.The
FILE_PROGRESSevent is properly added to theRAG_EVENTSobject with a consistent naming convention (rag:file-progress). This enables real-time progress feedback during file processing, which aligns with the PR's objective of implementing progress tracking.src/renderer/src/i18n/ja-JP/settings.json (1)
62-64: LGTM! Japanese translations properly integrated.The Japanese translations for the file size configuration settings are well-formed and consistent with the multilingual support being added across all locale files in this PR.
src/main/presenter/index.ts (1)
105-105: LGTM! Correct dependency injection for FilePresenter.The change properly passes
this.configPresenterto theFilePresenterconstructor, enabling it to access runtime configuration settings likemaxFileSize. This wiring is essential for the feature's dynamic file size limits.The constructor signature from the relevant code snippets confirms this is correct:
constructor(configPresenter: IConfigPresenter, fileValidationService?: IFileValidationService)src/renderer/settings/components/CommonSettings.vue (2)
6-6: LGTM! Clean integration of the new settings component.The
UploadFileSettingsSectioncomponent is properly imported and rendered in a logical location between web content settings and search assistant settings. The component appears to be self-contained, requiring no additional props or event handlers in this parent component.
54-54: LGTM! Import statement correctly added.The import for
UploadFileSettingsSectionis properly placed with the other settings section imports.src/renderer/src/i18n/fr-FR/settings.json (1)
62-64: LGTM! French translations for file size settings are clear and accurate.The new translation keys
fileMaxSizeandfileMaxSizeHintare well-translated and align with the UI functionality introduced by the UploadFileSettingsSection component.src/renderer/src/i18n/fa-IR/settings.json (1)
62-64: LGTM! Persian translations for file size settings added.The new translation keys are properly integrated into the fa-IR locale file to support the file upload size configuration UI.
src/renderer/src/i18n/pt-BR/settings.json (1)
62-64: LGTM! Portuguese (Brazil) translations for file size settings are appropriate.The translations are clear and align well with the new file upload configuration feature.
src/renderer/settings/components/KnowledgeFileItem.vue (1)
35-48: LGTM! Progress tracking UI enhancement improves user feedback.The addition of a tooltip showing progress percentage and counts (completed + error / total) provides clear real-time feedback during file processing. The computed
progressPercentcorrectly accounts for both completed and errored items in the progress calculation.src/renderer/src/i18n/ko-KR/settings.json (1)
62-64: LGTM! Korean translations for file size settings are clear.The translations properly support the new file upload size configuration feature in the Korean locale.
src/renderer/src/i18n/zh-HK/settings.json (1)
62-64: LGTM! Traditional Chinese translations for file size settings added.The translations support the new file upload configuration UI for the zh-HK locale. Note that this file also includes the
traceDebugEnabledkey.src/renderer/settings/components/common/UploadFileSettingsSection.vue (1)
1-63: LGTM! File size configuration UI is well-designed.The component provides a clear and intuitive interface for managing file upload size limits with:
- Visual display of current value with inline editing capability
- Increment/decrement buttons with 50 MB steps
- Proper min/max constraints (1-1024 MB)
- Good keyboard handling (Enter/Escape)
src/main/presenter/filePresenter/FilePresenter.ts (1)
28-30: LGTM! Dynamic file size configuration properly implemented.The addition of the
configPresenterdependency and themaxFileSizegetter enables runtime-configurable file upload limits. The default value of 30 MB aligns with the UI default in UploadFileSettingsSection.vue.Also applies to: 32-36
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/presenter/filePresenter/FilePresenter.ts(2 hunks)src/renderer/settings/components/common/UploadFileSettingsSection.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/settings/components/common/UploadFileSettingsSection.vue
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/presenter/filePresenter/FilePresenter.ts (3)
src/shared/types/presenters/legacy.presenters.d.ts (1)
IConfigPresenter(402-607)src/main/presenter/filePresenter/FileValidationService.ts (1)
IFileValidationService(14-18)test/mocks/electron.ts (1)
app(2-10)
🔇 Additional comments (3)
src/main/presenter/filePresenter/FilePresenter.ts (3)
7-7: LGTM!The import of
IConfigPresenteris appropriate and necessary for the new configurable file size feature.
24-24: LGTM!The private field declaration is correct and properly typed.
90-90: LGTM!The usage of
this.maxFileSizecorrectly invokes the getter, allowing the adapter to use the dynamically configured max file size.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/presenter/filePresenter/FilePresenter.ts (1)
7-7: Config-drivenmaxFileSizewiring looks good; consider light validation and unit clarity.Injecting
IConfigPresentervia the constructor and resolvingmaxFileSizethroughgetSetting<number>('maxFileSize') ?? 30MBis clean and keeps this logic configurable, and using??correctly preserves an explicit0value instead of falling back.To make this more robust against bad config, consider:
- Validating that the retrieved value is a finite, non-negative number.
- Optionally clamping it to a sane upper bound, or falling back to the default if invalid.
- Ensuring (and documenting) that the stored value uses the same units as the default (bytes here), so renderer/UI code doesn’t accidentally treat it as MB.
This is non-blocking, but would prevent surprising behavior if the setting is ever misconfigured.
Also applies to: 24-30, 32-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/presenter/filePresenter/FilePresenter.ts(2 hunks)src/renderer/settings/components/common/UploadFileSettingsSection.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/settings/components/common/UploadFileSettingsSection.vue
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/presenter/filePresenter/FilePresenter.ts (3)
src/shared/types/presenters/legacy.presenters.d.ts (1)
IConfigPresenter(402-607)src/main/presenter/filePresenter/FileValidationService.ts (1)
IFileValidationService(14-18)test/mocks/electron.ts (1)
app(2-10)
|
LGTM |
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.