From cd243ed6c3ba2d825cf6e0eaf7222043d07d7d86 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 3 Jun 2026 13:03:42 +0200 Subject: [PATCH] fix(search): address review feedback on multi-folder path filter - visualization.py: drop the CSV string-split branch. The Astrolabe PHP client sends path_prefixes as a JSON array, so only a list is accepted; any other shape is ignored rather than comma-split (which would corrupt folder names containing commas). - viz_routes.py: split the path_prefixes query param on newline (a comma is a valid POSIX path char; a newline is not) and pass None instead of [""] when the param is absent. - access_filter.py: widen build_base_filter_conditions' path_prefixes to Iterable[str] for consistency with normalize_path_prefixes. - ADR-027: document the newline delimiter (frontend/viz route) and JSON array (PHP->MCP body), and the PHP-side cap on list width. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/ADR-027-rich-search-filters.md | 11 +++++--- nextcloud_mcp_server/api/visualization.py | 28 +++++++------------- nextcloud_mcp_server/auth/viz_routes.py | 9 ++++--- nextcloud_mcp_server/search/access_filter.py | 2 +- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/docs/ADR-027-rich-search-filters.md b/docs/ADR-027-rich-search-filters.md index a8c70260..0312eebc 100644 --- a/docs/ADR-027-rich-search-filters.md +++ b/docs/ADR-027-rich-search-filters.md @@ -201,14 +201,17 @@ express: drops blanks, and de-dupes). `build_base_filter_conditions` adds a single `MatchText` to the `must` clause for one folder, and OR-s multiple folders via a nested `Filter(should=[...])` so a file under **any** selected folder matches while still AND-ing against the ACL/doc_type/date - conditions. Every search surface parses the list: the MCP tool (`nc_semantic_search`), the - visualization API (JSON body), and the viz route (CSV query param). + conditions. Every search surface parses the list: the MCP tool (`nc_semantic_search`) takes a + real `list[str]`, the visualization API takes a JSON array body, and the viz route takes a + **newline-separated** query param. Newline (not comma) is the on-the-wire delimiter because it + can't appear in a POSIX path, so folder names are never split mid-value. - **Frontend uses the native folder picker.** Instead of a free-text path input, the Astrolabe app opens Nextcloud's server-side folder browser via `getFilePickerBuilder()` from `@nextcloud/dialogs` (already a dependency — no `@nextcloud/vue` component-version coupling), configured directory-only + multi-select. Picked folders are real, validated server paths - (no typos), rendered as removable chips, and sent as a comma-separated `path_prefixes` list. The - Astrolabe PHP `ApiController`/`McpServerClient` forward the list to the MCP server. The control + (no typos), rendered as removable chips, and sent as a newline-joined `path_prefixes` value. The + Astrolabe PHP `ApiController` splits on newline (capping the list to bound the OR-filter width) + and `McpServerClient` forwards a JSON array to the MCP server. The control is enabled only when the **Files** doc type is in scope; an empty selection means "no filter". - **Phase 3 — tags (and optionally category).** Add a `tags: list[str]` payload field in `processor.py`, propagate Nextcloud system tags during scanning, trigger a re-index, then wire diff --git a/nextcloud_mcp_server/api/visualization.py b/nextcloud_mcp_server/api/visualization.py index 0c899645..6dd67ab7 100644 --- a/nextcloud_mcp_server/api/visualization.py +++ b/nextcloud_mcp_server/api/visualization.py @@ -231,17 +231,13 @@ async def unified_search(request: Request) -> JSONResponse: # ADR-027 Phase 2 path filter (files only); blank ⇒ no filter. Accept a # path_prefixes list (multi-folder) alongside the legacy single # path_prefix; normalize drops blanks and de-dupes. + # path_prefixes arrives as a JSON array (the Astrolabe PHP client sends + # a list); any other shape is ignored rather than guessed at. The legacy + # single path_prefix is folded in by normalize_path_prefixes. _path_prefixes_raw = body.get("path_prefixes") - if isinstance(_path_prefixes_raw, list): - _path_prefixes_list = _path_prefixes_raw - elif isinstance(_path_prefixes_raw, str): - _path_prefixes_list = _path_prefixes_raw.split(",") - else: - # Ignore any other JSON shape (number, object, null) rather than - # blowing up on .split — the legacy path_prefix still applies. - _path_prefixes_list = [] path_prefixes = normalize_path_prefixes( - body.get("path_prefix"), _path_prefixes_list + body.get("path_prefix"), + _path_prefixes_raw if isinstance(_path_prefixes_raw, list) else None, ) if not query: @@ -459,17 +455,13 @@ async def vector_search(request: Request) -> JSONResponse: # ADR-027 Phase 2 path filter (files only); blank ⇒ no filter. Accept a # path_prefixes list (multi-folder) alongside the legacy single # path_prefix; normalize drops blanks and de-dupes. + # path_prefixes arrives as a JSON array (the Astrolabe PHP client sends + # a list); any other shape is ignored rather than guessed at. The legacy + # single path_prefix is folded in by normalize_path_prefixes. _path_prefixes_raw = body.get("path_prefixes") - if isinstance(_path_prefixes_raw, list): - _path_prefixes_list = _path_prefixes_raw - elif isinstance(_path_prefixes_raw, str): - _path_prefixes_list = _path_prefixes_raw.split(",") - else: - # Ignore any other JSON shape (number, object, null) rather than - # blowing up on .split — the legacy path_prefix still applies. - _path_prefixes_list = [] path_prefixes = normalize_path_prefixes( - body.get("path_prefix"), _path_prefixes_list + body.get("path_prefix"), + _path_prefixes_raw if isinstance(_path_prefixes_raw, list) else None, ) # ADR-027 modified-date range filter. Accepts RFC 3339 / ISO 8601 # datetimes or Unix seconds; normalized to int Unix seconds. None ⇒ open. diff --git a/nextcloud_mcp_server/auth/viz_routes.py b/nextcloud_mcp_server/auth/viz_routes.py index 1f94d0c4..50ffcefa 100644 --- a/nextcloud_mcp_server/auth/viz_routes.py +++ b/nextcloud_mcp_server/auth/viz_routes.py @@ -148,12 +148,15 @@ async def vector_visualization_search(request: Request) -> JSONResponse: doc_types = doc_types_param.split(",") if doc_types_param else None # ADR-027 Phase 2 path filter (files only); blank ⇒ no filter. Accept a - # comma-separated path_prefixes list (multi-folder) plus the legacy single - # path_prefix; normalize_path_prefixes drops blanks and de-dupes. + # newline-separated path_prefixes list (multi-folder) plus the legacy single + # path_prefix; normalize_path_prefixes drops blanks and de-dupes. Newline is + # the delimiter because it can't appear in a POSIX path (unlike a comma), so + # folder names are never split mid-value. path_prefix = request.query_params.get("path_prefix") + _raw_prefixes = request.query_params.get("path_prefixes") path_prefixes = normalize_path_prefixes( path_prefix, - (request.query_params.get("path_prefixes") or "").split(","), + _raw_prefixes.split("\n") if _raw_prefixes else None, ) # Parse ADR-027 modified-date range filter. Accepts RFC 3339 / ISO 8601 diff --git a/nextcloud_mcp_server/search/access_filter.py b/nextcloud_mcp_server/search/access_filter.py index c0b82448..05a0d76a 100644 --- a/nextcloud_mcp_server/search/access_filter.py +++ b/nextcloud_mcp_server/search/access_filter.py @@ -234,7 +234,7 @@ def build_base_filter_conditions( modified_after: int | None = None, modified_before: int | None = None, path_prefix: str | None = None, - path_prefixes: list[str] | None = None, + path_prefixes: Iterable[str] | None = None, ) -> list[Condition]: """Build the common ``must`` conditions shared by every search algorithm.