feat(import): batch and folder import, as a snapshot with honest counts - #172
Merged
Merged
Conversation
Adding sources was one file at a time, and research material arrives as folders (#98). - scanFolder(): recursive, supported extensions only, hidden directories skipped, deterministic order, symlinks not followed. - KnowledgeService.addDocumentsFromPaths(): a batch where one file's failure does not abort the rest (each file still gets its own ingestion run, #95), and a path already in the notebook is skipped and reported rather than imported twice. - addFolder() = scan + batch. A snapshot, NOT a watch: files added to the folder later are not picked up — that is #158. - IPC knowledge:add-folder / add-files / select-folder. The renderer's multi-select and drag-and-drop both go through the same batch path, so dedup and skip reporting are one implementation. Electron 39 removed File.path, so the drop handler resolves paths through webUtils in preload. - The source list reports "Imported N · skipped M (already imported) · failed K" instead of a single total, and gains an Add folder item and a drop target. Verified: npm run typecheck; npm test (401 pass, incl. folder-scan fixture tests); npm run check:design; npm run build; eval baseline unchanged; electron . --smoke-test PASS (28 checks), including a real folder import that asserts unsupported files are skipped and that re-importing the same folder adds nothing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Batch and folder import (#98): add a folder recursively, import many files at once, and report honestly what happened to each.
Why?
Adding sources was one file at a time. Research material arrives as directories — a course folder, a paper collection, a project's docs — and importing 40 PDFs one dialog at a time is where a user stops importing. The folder import is explicitly a snapshot, not a watch; watching is #158.
Related issue
Fixes #98
Related to #95 (ingestion runs), #154
What changed
scanFolder()(src/main/services/ingestion/folderScan.ts): recursive, supported extensions only, hidden directories skipped, deterministic order, symlinks not followed. The extension list comes fromFileParserService.supportedExtensions(), so the scanner and the loaders cannot drift apart.KnowledgeService.addDocumentsFromPaths(): a batch where one file's failure does not abort the rest — each file still goes throughaddDocumentFromFile, so each gets its own ingestion run and failure record ([Feat] Ingestion runs: persistent pipeline state, retry, and non-destructive re-index #95). A path already in the notebook is skipped and reported, not imported twice.addFolder()= scan + batch. Files added to the folder later are not picked up.knowledge:add-folder,knowledge:add-files,knowledge:select-folder. The renderer's multi-select and drag-and-drop both go through the same batch path, so dedup and skip reporting are one implementation. Electron 39 removedFile.path, so the drop handler resolves paths throughwebUtilsin the preload.Imported N · skipped M (already imported) · failed K— instead of a single total.How was this tested?
npm run typecheck— passes.npm test— 401 pass, including folder-scan fixture tests (recursion, hidden dirs skipped, unsupported extensions skipped, case-insensitive, deterministic order).npm run check:design— no violations.npm run build— passes.electron . --smoke-test— PASS (28 checks), including a real folder import that asserts unsupported files are skipped and that re-importing the same folder adds nothing (snapshot semantics).Not verified
webUtils.getPathForFileis the documented Electron 39 API. The markup is covered by typecheck and the design guard only.Checklist
npm run typecheckpasses.npm run buildpasses.Desktop / build changes