feat(search): add an optional source_file filter to mempalace_search (#1815) - #1817
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new source_file filter to the semantic search functionality, allowing users to scope search results to a specific source file. The changes include adding input validation for the new filter, updating the ChromaDB and SQLite BM25 search backends to apply the filter, exposing the full source_path in results, and adding comprehensive tests. The feedback highlights a potential AttributeError in the validation function _sanitize_optional_source_file if a non-string value is passed, suggesting an explicit type check to handle this gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces an optional source_file filter to the semantic search functionality (mempalace_search tool and search_memories API). It adds input validation for the source_file parameter (allowing path characters but guarding against null bytes, lone surrogates, and excessive length), updates the ChromaDB and SQLite BM25 query builders to support filtering by the full stored path, exposes the full source_path alongside the basename source_file in search results, and includes comprehensive unit and integration tests. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…emPalace#1815) Expose source_file alongside wing/room on mempalace_search. build_where_filter generalizes to 0/1/2+ clauses and the filter threads through the main vector path, the index-mismatch fallback, the vector-disabled BM25/SQLite path, and the union lexical path so it never silently no-ops. Matching is on the exact full stored value; results now expose source_path (the full path) for round tripping, since the displayed source_file is a basename. The MCP schema gains the source_file property and a path-tolerant sanitizer rejects null bytes, lone surrogates, and overlong values. Fixes MemPalace#1815 Co-Authored-By: rendigua2025-gif <253093224+rendigua2025-gif@users.noreply.github.com>
…#1815) A JSON number or boolean passed for source_file is not coerced by the string schema type, so it reached _sanitize_optional_source_file and raised AttributeError from .strip() rather than a clean validation error. Add an isinstance guard that raises ValueError, which tool_search returns as a structured error. Regression test added.
005c401 to
c203aac
Compare
Fixes #1815
What does this PR do?
Adds an optional
source_filefilter to themempalace_searchMCP tool, nextto the existing
wing/roomfilters, so a query can be scoped to a singlesource document.
build_where_filter(wing, room, source_file)now builds 0, 1, or 2+ clauses(ChromaDB needs
$andonly for 2+ clauses; a single clause is returned bare).mempalace_searchcan take, so itnever silently no-ops when search degrades: the main vector drawer+closet
query, the index-mismatch fallback (
_query_drawers_with_filter_fallback),the vector-disabled BM25/SQLite path (
_bm25_only_via_sqlite, in both the SQLfilter and the Python post-filter), and the lexical
unioncandidate path.Mined drawers store the full path while results display only the basename, so
each result now also carries
source_path(the full stored value); a callerreads
source_pathfrom a result and passes it back as thesource_filefilter.
mempalace_searchschema gains asource_fileproperty (required:tools/callrejects arguments not declared in the schema). The responsefiltersblock echoes the appliedsource_file.source_fileis a path, not a name, so it skipssanitize_name(whichrejects
/and..) and uses a dedicated check: it is matched verbatim as ametadata / parameterized-SQL value and never touches the filesystem, so there
is no traversal risk, but null bytes, lone surrogates, and overlong values
are still rejected for parity with
sanitize_name.Out of scope (no
source_fileparameter, so nothing to bypass): the CLIsearchcommand and thelayers.*retrieval stack.How to test
End to end through the MCP tool:
source_filevalues (for example/a/x.mdand
/a/y.md).mempalace_searchwithsource_file="/a/x.md": only that source comesback,
filters.source_fileechoes it, and each hit'ssource_pathis thefull path.
source_filereturns both sources.Checklist
python -m pytest tests/ -v)ruff check .)