fix(rag): skip hidden and junk directories when indexing#5633
Conversation
index_personal_documents walked the whole tree with no pruning, so pointing RAG at a real-world folder silently swept in .obsidian/ plugin JS, .git/ internals, node_modules/, and __pycache__/ — multiplying indexing time and polluting retrieval with junk chunks. Prune hidden directories and well-known junk directories from the walk, and skip hidden files. The explicitly passed root is exempt, so a user who deliberately indexes a hidden directory still gets its contents.
…ed helper The #5559 fix pruned only VectorRAG.index_personal_documents (the vector index). The parallel keyword index built by PersonalDocsManager.refresh_index -> load_personal_index walked the same tree unpruned, so .obsidian/, .git/, node_modules/ etc. still swept into keyword retrieval and the file listing — the 'end-to-end' guarantee was only half true. Single-source the pruning policy in src/index_walk (prune_index_dirs + is_indexable_file) and use it from both walkers so they cannot drift again. The junk-dir match is now case-insensitive, so a Node_Modules on a case-insensitive filesystem is pruned too. Tests: keyword-path regressions covering hidden/junk dirs, hidden files, junk at depth (not just top level), case-insensitive junk, and the explicit-hidden- root exemption. The existing vector tests still pass against the shared helper.
|
pushed efed441. the fix was only half applied: the keyword index (load_personal_index, which refresh_index builds from) was still walking the tree unpruned, so .obsidian/node_modules content stayed in keyword retrieval and the file listing. single-sourced the prune policy in src/index_walk and wired both the vector and keyword walkers to it so they can't drift again. also made the junk-dir match case-insensitive, since Node_Modules on a case-insensitive filesystem slipped the old exact-match set. added keyword-path regressions covering hidden/junk dirs, hidden files, junk nested below the top level, and the case-insensitive case. |
|
@RaresKeY heads up on the CodeQL path-injection High here. it's pre-existing (the os.walk predates this PR) and a false positive: the add_directory route already confines the path with realpath + commonpath under PERSONAL_DIR before it reaches os.walk, CodeQL just doesn't count commonpath as a barrier. the other callers are local by design (stdio mcp, cli). looks safe to dismiss. |
RaresKeY
left a comment
There was a problem hiding this comment.
LGTM. The latest changes consistently filter hidden and junk directories across both vector and keyword indexing. Focused tests pass, and I found no actionable issues.
…v#5633) * fix(rag): skip hidden and junk directories when indexing (odysseus-dev#5559) index_personal_documents walked the whole tree with no pruning, so pointing RAG at a real-world folder silently swept in .obsidian/ plugin JS, .git/ internals, node_modules/, and __pycache__/ — multiplying indexing time and polluting retrieval with junk chunks. Prune hidden directories and well-known junk directories from the walk, and skip hidden files. The explicitly passed root is exempt, so a user who deliberately indexes a hidden directory still gets its contents. * fix(rag): prune hidden/junk dirs in the keyword index too, via a shared helper The odysseus-dev#5559 fix pruned only VectorRAG.index_personal_documents (the vector index). The parallel keyword index built by PersonalDocsManager.refresh_index -> load_personal_index walked the same tree unpruned, so .obsidian/, .git/, node_modules/ etc. still swept into keyword retrieval and the file listing — the 'end-to-end' guarantee was only half true. Single-source the pruning policy in src/index_walk (prune_index_dirs + is_indexable_file) and use it from both walkers so they cannot drift again. The junk-dir match is now case-insensitive, so a Node_Modules on a case-insensitive filesystem is pruned too. Tests: keyword-path regressions covering hidden/junk dirs, hidden files, junk at depth (not just top level), case-insensitive junk, and the explicit-hidden- root exemption. The existing vector tests still pass against the shared helper.
Summary
index_personal_documentswalks the target directory with no pruning, andDEFAULT_FILE_EXTENSIONSincludes.js,.json,.css,.html. Point RAG at a real-world folder and it indexes tool internals: an Obsidian vault's.obsidian/(2.6MB of minified plugin JS in the reporter's case), a repo's.git/,node_modules/,__pycache__/. Indexing time multiplies and retrieval fills with junk chunks.This PR prunes hidden directories and a small junk list (
node_modules,__pycache__,venv) from the walk in place, and skips hidden files. The explicitly passed root is exempt, so a user who deliberately indexes a hidden directory still gets its contents. Plus 3 hermetic regression tests (sameVectorRAG.__new__pattern astest_rag_remove_directory_scope.py); all 3 fail ondevwithout the fix.Target branch
dev, notmain.Linked Issue
Fixes #5559
Type of Change
Checklist
devuvicorn app:app+ a local ChromaDB atlocalhost:8100) and verified the change works end-to-end.How to Test
python -m pytest tests/test_rag_index_hidden_dirs.py: 3 tests covering hidden dirs, hidden files, junk dirs, and the explicit-hidden-root exemption. They fail ondevwithout the fix.End-to-end (what i ran): start ChromaDB and the app, then create a directory under
data/personal_docs/containing real content plus junk dirs:POST /api/personal/add_directorywith that path. Response on this branch:indexed_count: 2, failed_count: 0.Confirm in the vector store that only the two real files landed. Querying the
odysseus_rag_fastembedcollection's metadatas after the call returns exactlynotes/meeting.mdandreadme.mdas sources, none of the junk paths. Ondevthe same tree also embeds the.obsidian/,.git/,node_modules/,__pycache__/, andvenv/files.Also ran: full
python -m pytest(4673 passed, 3 skipped; the 5 failures are the docker-socket tests, which fail identically on cleandevon this machine, macOS with no docker socket) andpython -m compileall app.py core routes src services scripts tests.Scope notes
include_hiddenas optional; adding it means plumbing a flag through the route,PersonalDocsManager,RAGManager, and the UI, so i left it out to keep this small. Can follow up if wanted.dist/,build/,target/could join it, but those names collide with legitimate content folders more often.