Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions redisvl/extensions/router/semantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,15 @@ def _make_filter_queries(ids: list[str]) -> list[FilterQuery]:
queries = []

for id in ids:
if not id:
# `Tag(...) == ""` renders as the match-all `*`, so an empty id
# would match every reference in the index. Callers take the
# first row of each query's results, which turns this into
# returning -- and, from delete_route_references, deleting -- an
# arbitrary reference the caller never named.
raise ValueError(
"reference ids must be non-empty strings; received an empty id"
)
fe = Tag("reference_id") == id
fq = FilterQuery(
return_fields=["reference_id", "route_name", "reference"],
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/test_semantic_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
RoutingConfig,
)
from redisvl.redis.connection import is_version_gte
from redisvl.redis.utils import convert_bytes
from tests.conftest import skip_if_no_redis_search, skip_if_redis_version_below

pytestmark = pytest.mark.requires_hf
Expand Down Expand Up @@ -728,6 +729,26 @@ def test_get_route_references(semantic_router):
semantic_router.get_route_references()


def test_empty_reference_id_deletes_nothing(semantic_router):
# An empty id used to render the match-all `*`, and because the caller reads
# the first row of each query's results it removed an arbitrary reference
# rather than the one asked for.
prefix = semantic_router._index.schema.index.prefix
keys_before = set(
convert_bytes(semantic_router._index._redis_client.keys(f"{prefix}*"))
)

with pytest.raises(ValueError):
semantic_router.delete_route_references(reference_ids=[""])
with pytest.raises(ValueError):
semantic_router.get_route_references(reference_ids=[""])

keys_after = set(
convert_bytes(semantic_router._index._redis_client.keys(f"{prefix}*"))
)
assert keys_after == keys_before


def test_delete_route_references(semantic_router):
# Get references for a specific route
deleted = semantic_router.delete_route_references(route_name="greeting")
Expand Down
29 changes: 29 additions & 0 deletions tests/unit/test_semantic_router_queries.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""Hermetic tests for SemanticRouter's reference-id filter construction.

`_make_filter_queries` is a static method that touches neither Redis nor a
vectorizer, so its behaviour is testable without the router's integration
fixtures -- which matters because a defect here silently deletes data.
"""

import pytest

from redisvl.extensions.router.semantic import SemanticRouter


def test_reference_id_query_is_scoped_to_the_id():
(query,) = SemanticRouter._make_filter_queries(["abc123"])
assert str(query._filter_expression) == "@reference_id:{abc123}"


@pytest.mark.parametrize("empty", ["", None], ids=["empty_string", "none"])
def test_empty_reference_id_is_rejected_rather_than_matching_everything(empty):
# `Tag("reference_id") == ""` renders as `*`. Callers read the first row of
# each query's results, so a match-all here returns -- and, from
# delete_route_references, deletes -- a reference nobody named.
with pytest.raises(ValueError, match="non-empty"):
SemanticRouter._make_filter_queries([empty])


def test_one_empty_id_rejects_the_whole_batch():
with pytest.raises(ValueError, match="non-empty"):
SemanticRouter._make_filter_queries(["abc123", "", "def456"])
Loading