Skip to content

Commit 074e8bf

Browse files
authored
fix: honor distance_threshold=0 in semantic cache and message history (#650)
Fixes #649 ### Problem The per-call `distance_threshold` override was resolved with a truthiness check: ```python distance_threshold = distance_threshold or self._distance_threshold ``` `0.0` is falsy, so a caller passing `distance_threshold=0` (exact-match-only) had it silently replaced by the instance default and received fuzzy cache hits instead. `0` is a valid threshold everywhere else: `set_threshold()` accepts `0 <= x <= 2` and `VectorRangeQuery.set_distance_threshold()` rejects only negatives. The failure mode is a false cache hit so the caller gets a cached response to a prompt they never asked, believing fuzzy matching was off. ### Fix Fall back to the configured default only when the override is `None`. Applied in `SemanticCache.check()`, `SemanticCache.acheck()`, and `SemanticMessageHistory.get_relevant()`. ### Tests Added `test_check_with_zero_distance_threshold` and `test_acheck_with_zero_distance_threshold` to `tests/integration/test_llmcache.py`. Each stores a prompt, asserts a near-paraphrase does **not** hit at `distance_threshold=0`, and asserts it **does** hit at the cache default (so the test fails for the right reason rather than by accident). Verified against the real suite (Python 3.12, Redis 8.4 via the repo's compose fixture): - before the fix: both new tests fail with `AssertionError: assert [{'entry_id': ...}] == []` - after the fix: both pass - full `tests/integration/test_llmcache.py`: **62 passed** ### Scope note I left the `ttl = ttl or self._ttl` occurrences alone. A TTL of 0 isn't meaningful in Redis and the suite already rejects it (`test_bad_ttl`), so the falsy-fallback is harmless there. Happy to widen the scope if maintainers prefer consistency. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, targeted behavior fix in lookup paths with clear regression tests; no API or schema changes beyond honoring an already-valid threshold value. > > **Overview** > Fixes a bug where per-call **`distance_threshold`** overrides used `distance_threshold or self._distance_threshold`, so **`0`** (exact-match-only in Redis COSINE distance) was treated as unset and replaced by the instance default—causing **false cache hits** and fuzzy context when callers intended strict matching. > > **`SemanticCache.check()`**, **`SemanticCache.acheck()`**, and **`SemanticMessageHistory.get_relevant()`** now fall back to the configured default only when the override is **`None`**. > > Integration tests cover sync/async cache checks and semantic history retrieval: near-paraphrases must not match at **`distance_threshold=0`**, while the same queries still match at the default threshold. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5620780. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 788f368 commit 074e8bf

4 files changed

Lines changed: 63 additions & 6 deletions

File tree

redisvl/extensions/cache/llm/semantic.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,8 +398,10 @@ def check(
398398
if return_fields and not isinstance(return_fields, list):
399399
raise TypeError("Return fields must be a list of values.")
400400

401-
# Use overrides or defaults
402-
distance_threshold = distance_threshold or self._distance_threshold
401+
# Use overrides or defaults. Note: 0 is a valid distance threshold
402+
# (exact match only), so only fall back on None.
403+
if distance_threshold is None:
404+
distance_threshold = self._distance_threshold
403405

404406
# Vectorize prompt if not provided
405407
if vector is None and prompt is not None:
@@ -491,8 +493,10 @@ async def acheck(
491493
if return_fields and not isinstance(return_fields, list):
492494
raise TypeError("Return fields must be a list of values.")
493495

494-
# Use overrides or defaults
495-
distance_threshold = distance_threshold or self._distance_threshold
496+
# Use overrides or defaults. Note: 0 is a valid distance threshold
497+
# (exact match only), so only fall back on None.
498+
if distance_threshold is None:
499+
distance_threshold = self._distance_threshold
496500

497501
# Vectorize prompt if not provided
498502
if vector is None and prompt is not None:

redisvl/extensions/message_history/semantic_history.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,10 @@ def get_relevant(
231231
# Validate and normalize role parameter
232232
roles_to_filter = self._validate_roles(role)
233233

234-
# override distance threshold
235-
distance_threshold = distance_threshold or self._distance_threshold
234+
# override distance threshold. Note: 0 is a valid distance threshold
235+
# (exact match only), so only fall back on None.
236+
if distance_threshold is None:
237+
distance_threshold = self._distance_threshold
236238

237239
return_fields = [
238240
SESSION_FIELD_NAME,

tests/integration/test_llmcache.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,33 @@ def test_check_no_match(cache, vectorizer):
577577
assert len(check_result) == 0
578578

579579

580+
def test_check_with_zero_distance_threshold(cache):
581+
"""A per-call distance_threshold of 0 must be honored, not treated as unset.
582+
583+
Regression test: `distance_threshold or self._distance_threshold` treated the
584+
valid threshold 0 as falsy and silently fell back to the cache default, so
585+
callers requesting exact-match-only lookups received fuzzy matches.
586+
"""
587+
cache.store("what is the capital of france?", "paris")
588+
589+
# Semantically similar but not identical -> must NOT hit at threshold 0
590+
assert cache.check("what's the capital of france?", distance_threshold=0) == []
591+
592+
# Sanity check: the same query does hit at the cache's default threshold
593+
assert cache.check("what's the capital of france?") != []
594+
595+
596+
@pytest.mark.asyncio
597+
async def test_acheck_with_zero_distance_threshold(cache):
598+
"""Async variant: a per-call distance_threshold of 0 must be honored."""
599+
await cache.astore("what is the capital of france?", "paris")
600+
601+
assert (
602+
await cache.acheck("what's the capital of france?", distance_threshold=0)
603+
) == []
604+
assert (await cache.acheck("what's the capital of france?")) != []
605+
606+
580607
def test_check_invalid_input(cache):
581608
with pytest.raises(ValueError):
582609
cache.check()

tests/integration/test_message_history.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,30 @@ def test_semantic_add_and_get_relevant(semantic_history):
606606
bad_context = semantic_history.get_relevant("test prompt", top_k="3")
607607

608608

609+
@requires_hf
610+
def test_semantic_get_relevant_with_zero_distance_threshold(semantic_history):
611+
"""A per-call distance_threshold of 0 must be honored, not treated as unset.
612+
613+
Regression test: `distance_threshold or self._distance_threshold` treated the
614+
valid threshold 0 as falsy and silently fell back to the configured default,
615+
so callers requesting exact-match-only context received fuzzy matches.
616+
"""
617+
semantic_history.store(
618+
prompt="list of common fruits",
619+
response="apples, oranges, bananas, strawberries",
620+
)
621+
semantic_history.set_distance_threshold(0.5)
622+
623+
# Semantically similar but not identical -> must NOT match at threshold 0
624+
assert (
625+
semantic_history.get_relevant("list of typical fruits", distance_threshold=0)
626+
== []
627+
)
628+
629+
# Sanity check: the same query does match at the configured default
630+
assert semantic_history.get_relevant("list of typical fruits") != []
631+
632+
609633
@requires_hf
610634
def test_semantic_get_raw(semantic_history):
611635
semantic_history.store("first prompt", "first response")

0 commit comments

Comments
 (0)