mirror of
https://github.com/agentscope-ai/ReMe.git
synced 2026-10-27 06:15:20 +00:00
feat(search): add containment-style tags filter for multi-user scoping (#368)
Closes #368 ## What - Add containment-style tags filter to LocalFileStore._matches_search_filter with AND semantics (chunk must contain every expected tag). Matches the design note in the issue: frontmatter tags: [user:alice] plus the markdown chunker include_frontmatter_in_metadata=true lets multi-agent deployments scope recalls to a single user without per-user workspaces. - Promote tags context parameter in SearchStep and SearchV2Step to search_filter[tags], mirroring the existing start_date/end_date promotion. Accept both list[str] and comma-separated strings (trimmed). Explicit search_filter.tags takes precedence. - Add _tag_contains helper on LocalFileStore (covers list/set/tuple, scalar, and None inputs) and move the final two early-returns of _matches_search_filter to a combined boolean so pylint keeps the function's return-count under the limit. - Unit tests: containment semantics at the filter layer, list+CSV promotion in both SearchStep and SearchV2Step, precedence over top-level context. ## Why - The v0.4.x user-facing change removed an explicit user-scoping parameter. The documented workaround (separate workspaces per user) is heavyweight; the maintainer suggestion (store user:<name> in frontmatter tags) needed the corresponding search-side filter to be actionable in practice. - Using containment (AND) rather than exact equality keeps the frontmatter format natural: mix of user:<name>, project:<proj>, domain tags still works without calling multiple filters. - Using a reserved top-level tags key keeps the behavior explicit and the metadata semantics stable (metadata keys still use exact equality, which is what callers need for conversation_date style filters). ## Not in scope - Does NOT flip include_frontmatter_in_metadata default to true — that's a deploy-time config decision and changes memory usage. Docs update to note the knob is tracked separately. - No vector or keyword index changes; filtering stays post-hoc consistent with date/path/metadata filters today. ## Testing - [x] New unit tests (4) for filter semantics, list and CSV promotion, precedence, SearchV2Step — all PASS - [x] Existing search step test suite — 36/36 PASS (no regressions) - [x] Pre-commit: check-ast, black, flake8, pylint, pyroma, add-trailing-comma, whitespace, private-key — all PASS
This commit is contained in:
parent
550317c3bf
commit
047c1f39ee
4 changed files with 140 additions and 1 deletions
|
|
@ -693,6 +693,21 @@ class LocalFileStore(BaseFileStore):
|
|||
return actual in set(expected)
|
||||
return actual == expected
|
||||
|
||||
@classmethod
|
||||
def _tag_contains(cls, actual_tags, expected_tags) -> bool:
|
||||
"""Containment-style tag filter: chunk must contain every expected tag.
|
||||
|
||||
Matches the pattern described in the multi-user design note where
|
||||
frontmatter ``tags: [user:alice]`` is used for per-user scoping.
|
||||
"""
|
||||
expected = cls._as_filter_values(expected_tags)
|
||||
if not expected:
|
||||
return True
|
||||
if not isinstance(actual_tags, (list, tuple, set, frozenset)):
|
||||
return actual_tags in expected if actual_tags is not None else False
|
||||
actual = cls._as_filter_values(actual_tags)
|
||||
return expected.issubset(actual)
|
||||
|
||||
@staticmethod
|
||||
def _extract_date_from_path(path: str) -> str | None:
|
||||
"""Extract the date from a file path following the project path convention.
|
||||
|
|
@ -752,6 +767,7 @@ class LocalFileStore(BaseFileStore):
|
|||
return False
|
||||
|
||||
metadata_filter = dict(search_filter.get("metadata") or {})
|
||||
tag_filter = search_filter.get("tags")
|
||||
reserved = {
|
||||
"path",
|
||||
"paths",
|
||||
|
|
@ -763,12 +779,20 @@ class LocalFileStore(BaseFileStore):
|
|||
"start_date",
|
||||
"end_date",
|
||||
"strict_date_filter",
|
||||
"tags",
|
||||
}
|
||||
for key, value in search_filter.items():
|
||||
if key not in reserved:
|
||||
metadata_filter[key] = value
|
||||
|
||||
return all(cls._value_matches(chunk.metadata.get(key), value) for key, value in metadata_filter.items())
|
||||
passes_tags = True
|
||||
if tag_filter is not None:
|
||||
passes_tags = cls._tag_contains(chunk.metadata.get("tags"), tag_filter)
|
||||
|
||||
passes_metadata = all(
|
||||
cls._value_matches(chunk.metadata.get(key), value) for key, value in metadata_filter.items()
|
||||
)
|
||||
return passes_tags and passes_metadata
|
||||
|
||||
async def rebuild_links(self) -> None:
|
||||
"""Rebuild graph links via the underlying file graph."""
|
||||
|
|
|
|||
|
|
@ -177,6 +177,17 @@ class SearchStep(BaseStep):
|
|||
if value and date_key not in search_filter:
|
||||
search_filter[date_key] = value
|
||||
|
||||
# Promote top-level tags parameter for containment-style tag filtering.
|
||||
# Markdown frontmatter ``tags: [user:alice, project:foo]`` becomes
|
||||
# ``chunk.metadata["tags"]`` when the chunker's
|
||||
# ``include_frontmatter_in_metadata=true``; callers that need strict
|
||||
# user isolation should also enable that chunker option.
|
||||
tags_value = self.context.get("tags")
|
||||
if tags_value is not None and "tags" not in search_filter:
|
||||
if isinstance(tags_value, str):
|
||||
tags_value = [t.strip() for t in tags_value.split(",") if t.strip()]
|
||||
search_filter["tags"] = list(tags_value) if not isinstance(tags_value, list) else tags_value
|
||||
|
||||
# Validate and normalize date filters before they reach file_store.
|
||||
# _matches_search_filter does lexicographic string comparison against
|
||||
# path_date (always a canonical YYYY-MM-DD), so raw caller values like
|
||||
|
|
|
|||
|
|
@ -135,6 +135,16 @@ class SearchV2Step(_ToolContextDedupMixin, BaseStep):
|
|||
if value and date_key not in search_filter:
|
||||
search_filter[date_key] = value
|
||||
|
||||
# Promote top-level tags parameter for containment-style tag filtering.
|
||||
# When ``include_frontmatter_in_metadata=true`` on the markdown chunker,
|
||||
# frontmatter ``tags: [user:alice]`` propagates to chunk metadata and
|
||||
# can be used for per-user scoping without per-user workspaces.
|
||||
tags_value = self.context.get("tags")
|
||||
if tags_value is not None and "tags" not in search_filter:
|
||||
if isinstance(tags_value, str):
|
||||
tags_value = [t.strip() for t in tags_value.split(",") if t.strip()]
|
||||
search_filter["tags"] = list(tags_value) if not isinstance(tags_value, list) else tags_value
|
||||
|
||||
# Validate and normalize date filters before they reach file_store.
|
||||
# _matches_search_filter does lexicographic string comparison against
|
||||
# path_date (always a canonical YYYY-MM-DD), so raw caller values like
|
||||
|
|
|
|||
|
|
@ -984,3 +984,97 @@ def test_search_step_non_strict_date_filter_not_in_search_filter():
|
|||
assert "strict_date_filter" not in sf
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
|
||||
def _chunk_with_tags(chunk_id: str, path: str, text: str, tags: list[str] | None) -> FileChunk:
|
||||
chunk = _chunk(chunk_id, path, text, "keyword", 5.0)
|
||||
if tags is not None:
|
||||
chunk.metadata["tags"] = tags
|
||||
return chunk
|
||||
|
||||
|
||||
def test_matches_search_filter_supports_containment_tag_filter():
|
||||
"""``tags`` uses containment semantics so ``tags: [user:alice]`` frontmatter scopes searches."""
|
||||
# pylint: disable=protected-access
|
||||
from reme.components.file_store.local_file_store import LocalFileStore
|
||||
|
||||
user_a = _chunk_with_tags("a", "daily/a.md", "alice's note", ["user:alice", "project:foo"])
|
||||
user_b = _chunk_with_tags("b", "daily/b.md", "bob's note", ["user:bob", "project:foo"])
|
||||
no_tags = _chunk_with_tags("n", "daily/n.md", "untagged", None)
|
||||
|
||||
# Single required tag — exact user scoping (primary use case from #368)
|
||||
assert LocalFileStore._matches_search_filter(user_a, {"tags": "user:alice"}) is True
|
||||
assert LocalFileStore._matches_search_filter(user_b, {"tags": "user:alice"}) is False
|
||||
|
||||
# List required: chunk must contain *every* expected tag (AND semantics)
|
||||
assert LocalFileStore._matches_search_filter(user_a, {"tags": ["user:alice", "project:foo"]}) is True
|
||||
assert LocalFileStore._matches_search_filter(user_a, {"tags": ["user:alice", "project:bar"]}) is False
|
||||
|
||||
# Comma-separated list passed by the filter is normalized to a set
|
||||
assert LocalFileStore._matches_search_filter(user_a, {"tags": ["user:alice"]}) is True
|
||||
assert LocalFileStore._matches_search_filter(no_tags, {"tags": "user:alice"}) is False
|
||||
|
||||
# No filter -> passes
|
||||
assert LocalFileStore._matches_search_filter(user_a, None) is True
|
||||
assert LocalFileStore._matches_search_filter(user_a, {}) is True
|
||||
|
||||
|
||||
def test_search_step_tags_context_promoted_into_search_filter_list_and_csv():
|
||||
"""``tags`` context value is promoted to ``search_filter["tags"]``; CSV string is split."""
|
||||
|
||||
async def run():
|
||||
hit = _chunk_with_tags("hit", "daily/a.md", "text", ["user:alice"])
|
||||
store = FakeSearchStore(keyword_results=[hit])
|
||||
step = SearchStep(file_store=store, expand_links=False)
|
||||
|
||||
list_ctx = RuntimeContext(query="hello", limit=5, tags=["user:alice", "project:foo"])
|
||||
csv_ctx = RuntimeContext(query="hello", limit=5, tags=" user:alice , project:foo ")
|
||||
|
||||
await step(list_ctx)
|
||||
assert all(call[3]["tags"] == ["user:alice", "project:foo"] for call in store.calls)
|
||||
store.calls.clear()
|
||||
|
||||
await step(csv_ctx)
|
||||
assert all(call[3]["tags"] == ["user:alice", "project:foo"] for call in store.calls)
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
|
||||
def test_search_step_tags_in_search_filter_takes_precedence_over_context():
|
||||
"""Explicit ``search_filter.tags`` wins over top-level ``tags`` context."""
|
||||
|
||||
async def run():
|
||||
hit = _chunk("hit", "daily/a.md", "text", "keyword", 3.0)
|
||||
store = FakeSearchStore(keyword_results=[hit])
|
||||
step = SearchStep(file_store=store, expand_links=False)
|
||||
ctx = RuntimeContext(
|
||||
query="hello",
|
||||
limit=5,
|
||||
tags=["user:alice"],
|
||||
search_filter={"tags": ["user:bob"]},
|
||||
)
|
||||
|
||||
await step(ctx)
|
||||
|
||||
for _, _, _, sf in store.calls:
|
||||
assert sf["tags"] == ["user:bob"]
|
||||
|
||||
asyncio.run(run())
|
||||
|
||||
|
||||
def test_search_v2_step_tags_context_promoted_into_search_filter():
|
||||
"""SearchV2Step also promotes the ``tags`` context parameter into the filter envelope."""
|
||||
|
||||
async def run():
|
||||
hit = _chunk_with_tags("hit", "daily/a.md", "text", ["user:alice"])
|
||||
store = FakeSearchStore(keyword_results=[hit])
|
||||
step = SearchV2Step(file_store=store, expand_links=False)
|
||||
ctx = RuntimeContext(query="hello", limit=5, tags=["user:alice"])
|
||||
|
||||
await step(ctx)
|
||||
|
||||
assert len(store.calls) == 2
|
||||
for _, _, _, sf in store.calls:
|
||||
assert sf["tags"] == ["user:alice"]
|
||||
|
||||
asyncio.run(run())
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue