Skip to content

perf: run SemanticCache.check() as KNN with a client-side threshold (FDE-85) - #754

Open
vishal-bala wants to merge 2 commits into
mainfrom
feat/FDE-83-otel/01-FDE-85-knn-check
Open

vishal-bala wants to merge 2 commits into
mainfrom
feat/FDE-83-otel/01-FDE-85-knn-check

Conversation

@vishal-bala

@vishal-bala vishal-bala commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Motivation

SemanticCache.check() builds a VectorRangeQuery, so Redis discards every candidate beyond distance_threshold and a miss says nothing about how close it came. The nearest distance on a miss is what makes the threshold tunable, and it is the signal the OpenTelemetry instrumentation stacked on this change reports. Running the lookup as a KNN query with the threshold applied in Python provides that distance, and is faster when a filter is applied.

Changes

KNN lookup with the threshold applied in Python

check() and acheck() build a VectorQuery for num_results candidates with the same filter, return fields and dtype as before, then keep the candidates whose distance is at most the threshold. The comparison is inclusive, as the range query's radius was. Results, ordering and the returned shape are unchanged for callers. Query construction and the threshold filter are shared helpers, so the sync and async paths cannot drift.

The two queries are equivalent because the cache index is always FLAT, so both searches are exhaustive. Measured on Redis 8.4.7 (FLAT, cosine, 768-dimension float32 vectors), with 300 distinct queries repeated ten times per row:

Cache size and filter Range query (median) KNN with a client-side threshold
1,000 entries, no filter 243.0 µs +7.0 µs (+2.9%)
1,000 entries, tag filter matching 10% 245.6 µs −32.5 µs (−13.2%)
10,000 entries, no filter 825.3 µs +18.7 µs (+2.3%)
10,000 entries, tag filter matching 10% 815.7 µs −295.0 µs (−36.2%)

Hit and miss decisions agreed in all 3,000 samples per row, and in 200 of 200 queries placed within 2×10⁻⁶ of the threshold. The filtered speed-up is structural: the KNN query applies the filter first and compares vectors only within the matching subset, whereas the range query scans the whole index and intersects the result with the filter.

Validation and hardening kept

  • A per-call distance_threshold is validated as the range query validated it: TypeError for a non-number and ValueError for a negative value.
  • An id-only row from the Redis 8.8+ expiry race still reaches _process_cache_results, which skips it with its warning, rather than raising in the threshold filter.

Minor

  • A comment on the FLAT setting in the cache schema records that an HNSW option would make hits approximate.
  • Two stale # type: ignore comments on return_fields are removed.
  • An integration test, on the sync and async paths, checks a candidate exactly at the threshold and one just beyond it with num_results=3. The invalid-threshold cases join the existing invalid-input tests, and a hermetic test covers the id-only row.

Notes

The client compares the distance as Redis returns it, rounded to 12 significant digits, whereas the range query compared the unrounded value on the server. A candidate within about 10⁻¹² of the threshold can therefore fall on the other side of it. In practice this arises only when a caller passes a previously returned distance back as the threshold, and the new behaviour is the self-consistent one.

This is the bottom of a stack whose next PR adds the OpenTelemetry foundation. It is useful on its own and can merge first.

Release Notes

SemanticCache.check() and acheck() run as a KNN query with the distance threshold applied client-side. Lookups with a selective filter expression take up to about a third less time, and unfiltered lookups take 2–3% longer.


Note

Medium Risk
Changes core cache lookup semantics and performance for all check/acheck callers; behavior is intended to match on FLAT indexes but threshold comparison now uses Redis-reported distance precision client-side.

Overview
SemanticCache check() and acheck() no longer use a Redis range query with distance_threshold on the server. They issue a KNN VectorQuery for num_results candidates (same filters and return fields), then apply the threshold in Python via _within_threshold, so the nearest distance is available even on a miss and filtered lookups can be faster.

Sync and async paths share _build_check_query and _validate_distance_threshold for per-call overrides (type/range errors match prior behavior). Id-only rows from the Redis 8.8+ expiry race still pass through to _process_cache_results for the existing skip path. The cache schema comment documents why the index stays FLAT (exact KNN + threshold).

Tests cover inclusive threshold boundaries (sync/async), invalid distance_threshold, and threshold filtering with id-only candidates.

Reviewed by Cursor Bugbot for commit 27ea055. Bugbot is set up for automated code reviews on this repo. Configure here.

check() and acheck() searched with a VectorRangeQuery, so Redis
discarded every candidate beyond distance_threshold and a miss carried
no information about how close it came. They now run a KNN VectorQuery
for num_results candidates and keep those at or within the threshold in
Python, inclusive as the range radius was.

On the FLAT index the cache always uses, both queries are exhaustive,
so hit and miss decisions match. Measured on Redis 8.4.7 they agreed on
3,000 of 3,000 samples per configuration and on 200 of 200 queries
within 2e-6 of the threshold. The one theoretical difference is that
the client compares the distance as Redis returns it, rounded to 12
significant digits, so a distance within about 1e-12 of the threshold
can now fall on the other side of it.

Unfiltered lookups take 2-3% longer. With a tag filter matching 10% of
entries they take 13% less time at 1,000 entries and 36% less at
10,000, because KNN compares vectors only within the filtered subset
where the range query scanned the whole index. The nearest candidate's
distance is now in hand on every lookup, which the planned
OpenTelemetry instrumentation reports.

A per-call distance_threshold is still validated as the range query
validated it: TypeError for a non-number, ValueError for a negative
value. An id-only row (the Redis 8.8+ expiry race) still reaches
_process_cache_results and is skipped there rather than raising. Query
construction and the threshold filter are shared by the sync and async
paths.
@vishal-bala
vishal-bala added this pull request to stack #756 October 13, 2026 16:21
@vishal-bala
vishal-bala marked this pull request as ready for review October 14, 2026 08:08
@vishal-bala vishal-bala added the auto:performance Improve performance of an existing feature label Oct 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto:performance Improve performance of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant