diff --git a/redisvl/extensions/router/semantic.py b/redisvl/extensions/router/semantic.py index f2befc31..7ccba363 100644 --- a/redisvl/extensions/router/semantic.py +++ b/redisvl/extensions/router/semantic.py @@ -437,22 +437,20 @@ def _build_aggregate_request( aggregation_func = reducers.avg aggregate_query = str(vector_range_query).split(" RETURN")[0] - aggregate_request = ( + # FT.AGGREGATE runs its steps in the order given, and redis-py emits them + # in call order. FILTER must precede SORTBY ... MAX, or the max_k closest + # routes are kept before their thresholds are checked. + return ( AggregateRequest(aggregate_query) .group_by( "@route_name", # type: ignore aggregation_func("vector_distance").alias("distance"), ) + .filter(self._distance_threshold_filter()) .sort_by("@distance", max=max_k) # type: ignore .dialect(2) ) - filter = self._distance_threshold_filter() - - aggregate_request.filter(filter) - - return aggregate_request - def _get_route_matches( self, vector: list[float], diff --git a/tests/integration/test_semantic_router.py b/tests/integration/test_semantic_router.py index d6d9ba74..b1e20edd 100644 --- a/tests/integration/test_semantic_router.py +++ b/tests/integration/test_semantic_router.py @@ -616,6 +616,39 @@ def test_routes_different_distance_thresholds_get_one( router.delete() +def test_threshold_filter_applies_before_max_k_truncation( + client, routes, hf_vectorizer, redis_test_name +): + """A closer route that misses its own threshold must not crowd out a + farther one that meets its threshold.""" + skip_if_no_redis_search(client) + # With all-mpnet-base-v2 and avg aggregation, "hello" sits ~0.07 from + # greeting and ~0.60 from farewell. (Under min, greeting would be ~0.) + routes[0].distance_threshold = 0.01 + routes[1].distance_threshold = 0.8 + + router = SemanticRouter( + name=redis_test_name("test_threshold_filter_before_max_k"), + routes=routes, + routing_config=RoutingConfig( + max_k=1, aggregation_method=DistanceAggregationMethod.avg + ), + redis_client=client, + overwrite=True, + vectorizer=hf_vectorizer, + ) + try: + assert router("hello").name == "farewell" + + # max_k comes from the routing config; route() always uses max_k=1. + assert [match.name for match in router.route_many("hello")] == ["farewell"] + finally: + with suppress(Exception): + router.clear() + with suppress(Exception): + router.delete() + + def test_add_delete_route_references(semantic_router): skip_if_redis_version_below(semantic_router._index.client, "7.0.0")