Stop embeddings_test sharing one dense vector with every other test - #748
Merged
Conversation
vec(dim) was sin(i / dim) and vec(dim, seed) was sin((i + seed) / dim): DENSE, and IDENTICAL across every test that asked for the same dim. That is a distance-0 clique in the shared pgvector HNSW index, and under concurrent load the graph walk cannot navigate a clique of exact ties - it non-deterministically misses exact matches. KB 4e5ccb17 and 0bdadd52 record the mechanism; it is the same reason DataCase's default MockEmbeddingClient stub keys its vector on tenant_id instead of returning a constant. Both helpers now ride test_vec/2, so a module's rows sit on its OWN sparse dimensions (seeded per test by Process.put(:test_vec_axis, ...) in DataCase.setup). vec/2 keeps its N-distinct contract by scaling a component on the test's own orthogonal window - no call site depends on a particular similarity between those vectors, only that they are distinct and findable. The exposed assertion is the dimension-migration test at the end of the file: it stores a row with vec(768) and queries with the same vector through VectorSearch.index_safe_dimension_knn_base, asserting exactly one hit. A trapped walk returns [] there, which reads as a broken read path rather than as this. Sibling of the snippet-backfill fix, on its own branch because it is a different module with its own dimension-migration assertions. NOT "dead tuples crowd out the live row" - KB c6fd1e5d lists that as a dead theory, disproved by probe: pgvector skips invisible tuples and keeps scanning past 2000 rolled-back ties. That article opens by saying this family has been misdiagnosed twice, each time costing reverted fixes, so the reasoning is recorded next to the helpers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sibling of #747. Same defect class, different module.
The defect
vec(dim)wassin(i / dim)andvec(dim, seed)wassin((i + seed) / dim)— dense,and identical across every test that asked for the same dim. That is a distance-0 clique
in the shared pgvector HNSW index: under concurrent load the graph walk cannot navigate a
clique of exact ties and non-deterministically misses exact matches (KB
4e5ccb17,0bdadd52). It is the same reasonDataCase's defaultMockEmbeddingClientstub keys itsvector on
tenant_idinstead of returning a constant.Not "dead tuples crowd out the live row" — KB
c6fd1e5dlists that as a dead theory,disproved by probe (pgvector skips invisible tuples and keeps scanning past 2000 rolled-back
ties). That article opens by noting the family has been misdiagnosed twice, each time
costing reverted fixes, so the reasoning is recorded next to the helpers.
The exposed assertion
The dimension-migration test at the end of the file stores a row with
vec(768)and querieswith the same vector through
VectorSearch.index_safe_dimension_knn_base, asserting exactlyone hit. A trapped walk returns
[]there — which reads as a broken read path rather thanas this.
The fix
Both helpers ride
test_vec/2, so this module's rows sit on its own sparse dimensions(seeded per test by
Process.put(:test_vec_axis, …)inDataCase.setup).vec/2keeps itsN-distinct contract by scaling a component on the test's own orthogonal window — no call
site depends on a particular similarity between those vectors, only that they are distinct
and findable.
Free, like #747: this is not the
@moduletag :vacuum_vector_indexesroute, which measured+50% on the whole suite for one module and 18.6x for 24.
Verification
Full gate green through the pre-commit hook: 7827 tests, 0 failures (86 excluded), credo
--strictclean, dialyzer clean. The file's own 34 tests pass.Review
Reviewed inline and against the KB, which is what supplied the mechanism and ruled out the
one I would otherwise have written down. Two helper functions in one test file.