diff --git a/CHANGELOG.md b/CHANGELOG.md index fc623cc..96d6b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`resetRadiusThreshold()` workaround for the radius threshold leak** (#200) + - A radius-filtered Flat/IVF query poisons the thread-local search context: subsequent radius-less queries (refiner, plain, any field with the same index type) return only the radius-filtered subset, with no error. Upstream zvec caches the threshold on a `thread_local` context (`zvec/src/core/interface/index.cc`) and its Flat/IVF `reset()` is a no-op; radius is gated by `if (radius > 0.0f)` in `flat_index.cc`/`ivf_index.cc`. + - **`ZVec::resetRadiusThreshold(string $fieldName, array $vector)`** runs a throwaway topk-1 query with `radius = FLT_MAX` (`ZVec::RADIUS_THRESHOLD_RESET`) on the same field, which overwrites the leaked threshold and restores unfiltered behavior for later queries on that thread. + - Works for all metrics: for normalizing metrics (IP, sign-flip denormalization) radius filters docs with similarity below `-radius` (e.g. opposite vectors), so the leak can occur there too — verified: `resetRadiusThreshold()` restores the missing negative-similarity docs. For corpora where every similarity is ≥ `-radius`, the call is a harmless no-op. + - Regression test `test_radius_threshold_leak.phpt` documents the leak and the recovery. + - **`fetch()` with `outputFields` parameter** (#176) - `ZVecCollection::fetch()` now accepts an optional output-fields array to select which scalar columns are returned: `fetch(['pk1', 'pk2'], ['name', 'score'])`. - BC-compatible: the legacy variadic form `fetch('pk1', 'pk2')` (all fields) still works unchanged. diff --git a/docs/helpers/decisions.md b/docs/helpers/decisions.md index b7e82a7..4363dcc 100644 --- a/docs/helpers/decisions.md +++ b/docs/helpers/decisions.md @@ -73,3 +73,29 @@ present-null). Note: the Python SDK (pybind `Fetch`) does **not** apply this normalization — PHP intentionally follows the C API here. **Reference:** issue #192. + +--- + +### resetRadiusThreshold(): caller-driven purge instead of automatic mitigation + +**Decision:** issue #200 (radius threshold leak) is mitigated with an +explicit `ZVecCollection::resetRadiusThreshold(string $fieldName, array +$vector)` + `ZVec::RADIUS_THRESHOLD_RESET` (3.4028235e38) constant, not by +transparently fixing every radius-less query in the bindings. + +**Rationale:** the leak lives in the upstream zvec core (thread-local +context, no-op Flat/IVF reset); hiding it in `queryVector()` would need +per-call purge queries (cost) and could change result semantics. The +explicit call documents the upstream bug at the API surface, costs one +topk-1 query. Empirically verified against the built library for L2, IVF, +COSINE, MIPSL2 and IP (incl. negative-similarity docs): the purge restores +unfiltered results on every metric; it is a no-op only when no doc's +similarity is below `-radius`. For IP, radius filters docs with similarity +< `-radius` (sign-flip denormalization: engine converts radius to internal +distance, `set_threshold()` denormalizes it back — `index_context.h:235- +241`), so with opposite vectors the leak occurs there too. +`RADIUS_THRESHOLD_RESET` survives every metric's denormalization (sign +flip → -FLT_MAX, cosine `-= 1` → FLT_MAX in float32) and mirrors +`reset_threshold()` (FLT_MAX), which is unreachable from the C API. + +**Reference:** issue #200. diff --git a/docs/helpers/faq.md b/docs/helpers/faq.md index 9fee8b0..5b629a3 100644 --- a/docs/helpers/faq.md +++ b/docs/helpers/faq.md @@ -109,3 +109,28 @@ output (rocksdb_context.cc, inverted_indexer.cc, id_map.cc) — failing block, before the `finally` cleanup removes the directory. **Reference:** issue #192 (new test initially failed on shutdown noise). + +--- + +### Radius query poisons later queries on the same thread (Flat/IVF) + +**Problem:** one `setRadius(>0)` query makes every later radius-less query +on the same thread (refiner, plain, any field with the same index type, +even other collections) return only the radius-filtered subset — silently. +Upstream caches the threshold on a `thread_local` context shared per index +type (`zvec/src/core/interface/index.cc:33-35`) and Flat/IVF `reset()` is +a no-op; the `if (radius > 0.0f)` gate in `flat_index.cc:63` blocks +resetting it to 0. + +**Solution:** call `$c->resetRadiusThreshold($field, $queryVector)` after +radius-filtered queries. It runs a throwaway topk-1 query with +`radius = FLT_MAX`, which overwrites the stale threshold. Use the +`ZVec::RADIUS_THRESHOLD_RESET` constant, not `PHP_FLOAT_MAX` — the latter +overflows to `inf` in float32 (untested upstream semantics); FLT_MAX is +what upstream's own `reset_threshold()` uses. For IP metrics radius +filters docs with similarity below `-radius` (e.g. opposite vectors), so +the leak CAN occur there too — verified: `resetRadiusThreshold()` restores +the missing negative-similarity docs. With corpora where every similarity +is ≥ `-radius`, the call is a harmless no-op. + +**Reference:** issue #200, `tests/test_radius_threshold_leak.phpt`. diff --git a/src/ZVec.php b/src/ZVec.php index 2a5fd26..f6ac940 100644 --- a/src/ZVec.php +++ b/src/ZVec.php @@ -1076,6 +1076,19 @@ public function fetch(array|string|bool ...$args): array */ public const BYTES_PER_MB = 1048576; + /** + * Radius used by resetRadiusThreshold() to overwrite a stale threshold + * leaked into the thread-local Flat/IVF search context (issue #200). + * + * Maximum finite float (what upstream's reset_threshold() itself uses), + * chosen over PHP_FLOAT_MAX: the latter overflows to inf in float32, + * and it must survive every metric's denormalization (sign flip for + * IP → -FLT_MAX, cosine `-= 1` → FLT_MAX) and filter nothing. + * + * Value: 3.4028235e38 (FLT_MAX) + */ + public const RADIUS_THRESHOLD_RESET = 3.4028235e38; + /** * Default HNSW parameter: M (max connections per node). * @@ -1653,6 +1666,39 @@ public function queryVector(ZVecVectorQuery $query): array return self::parseQueryResult($result); } + /** + * Clear a stale radius threshold leaked into the thread-local search + * context by an earlier radius-filtered Flat/IVF query (issue #200). + * + * Upstream zvec caches the threshold on a thread-local context shared by + * every index of the same type in the process, and its Flat/IVF context + * reset() is a no-op — so one setRadius() > 0 query makes all later + * radius-less queries on that thread (refiner, plain, any field with the + * same index type) return only the radius-filtered subset. + * + * Pass the field and query vector of the radius query that caused the + * leak; a throwaway topk-1 query with a maximum radius overwrites the + * leaked threshold with the neutral FLT_MAX. Its result is meaningless + * and is discarded. For metrics that denormalize with a sign flip (IP) + * radius filters docs with similarity below -radius (e.g. opposite + * vectors), so the leak can occur there too — verified: this call + * restores the missing negative-similarity docs. + * + * Dense vector fields only: the purge query is always issued as a dense + * fp32 vector; sparse fields cannot be purged through this method. + * + * @param float[]|int[] $vector The query vector of the leaking radius query + * @throws ZVecException On FFI error + */ + public function resetRadiusThreshold(string $fieldName, array $vector): void + { + $this->checkClosed(); + + $query = new ZVecVectorQuery($fieldName, $vector); + $query->setTopk(1)->setRadius(self::RADIUS_THRESHOLD_RESET); + $this->queryVector($query); + } + /** * GroupBy query using a native ZVecGroupByVectorQuery object. * diff --git a/tests/test_radius_threshold_leak.phpt b/tests/test_radius_threshold_leak.phpt new file mode 100644 index 0000000..a0f6d94 --- /dev/null +++ b/tests/test_radius_threshold_leak.phpt @@ -0,0 +1,92 @@ +--TEST-- +Radius threshold leak: radius query poisons subsequent refiner/plain queries, resetRadiusThreshold() restores them (#200) +--SKIPIF-- + +--FILE-- + $d->getPk(), $results)); +} + +try { + $schema = new ZVecSchema('radius_leak'); + $schema->addVectorFp32('vf', dimension: 4, metricType: ZVecSchema::METRIC_L2); + + $c = ZVec::create($path, $schema); + $c->createIndex('vf', ZVecIndexParams::forFlat(ZVecSchema::METRIC_L2)); + + // Squared L2 distances from query [1,0,0,0]: doc1=0, doc2=0.25, doc3=4 + $vecs = [ + 'doc1' => [1.0, 0.0, 0.0, 0.0], + 'doc2' => [1.5, 0.0, 0.0, 0.0], + 'doc3' => [3.0, 0.0, 0.0, 0.0], + ]; + $docs = []; + foreach ($vecs as $pk => $v) { + $docs[] = (new ZVecDoc($pk))->setVectorFp32('vf', $v); + } + $c->insert(...$docs); + $c->flush(); + $c->optimize(); + + $qv = [1.0, 0.0, 0.0, 0.0]; + + // Sanity: radius query filters as expected + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams()->setRadius(0.3); + echo 'radius 0.3: ', pks($c->queryVector($q)), "\n"; + + // The leak (#200): after a radius query, refiner and plain queries on the + // same thread return only the radius-filtered subset. + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams()->setUsingRefiner(true); + echo 'refiner after radius: ', pks($c->queryVector($q)), "\n"; + + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams(); + echo 'plain after radius: ', pks($c->queryVector($q)), "\n"; + + // resetRadiusThreshold() purges the stale threshold + $c->resetRadiusThreshold('vf', $qv); + + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams()->setUsingRefiner(true); + echo 'refiner after reset: ', pks($c->queryVector($q)), "\n"; + + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams(); + echo 'plain after reset: ', pks($c->queryVector($q)), "\n"; + + // Radius filtering still works after the reset + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams()->setRadius(0.3); + echo 'radius after reset: ', pks($c->queryVector($q)), "\n"; + + // ... and the reset clears a fresh leak again + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams(); + echo 'plain after 2nd leak: ', pks($c->queryVector($q)), "\n"; + + $c->resetRadiusThreshold('vf', $qv); + $q = (new ZVecVectorQuery('vf', $qv))->setTopk(10)->setFlatParams(); + echo 'plain after 2nd reset: ', pks($c->queryVector($q)), "\n"; + + echo "ALL TESTS PASSED\n"; +} finally { + if (isset($c)) { try { $c->destroy(); } catch (Exception $e) {} } + exec("rm -rf " . escapeshellarg($path)); +} +?> +--EXPECT-- +radius 0.3: doc1,doc2 +refiner after radius: doc1,doc2 +plain after radius: doc1,doc2 +refiner after reset: doc1,doc2,doc3 +plain after reset: doc1,doc2,doc3 +radius after reset: doc1,doc2 +plain after 2nd leak: doc1,doc2 +plain after 2nd reset: doc1,doc2,doc3 +ALL TESTS PASSED