fix(cluster): correct the vector geometry and stop truncating candidates - #30
Merged
Conversation
Duplicate detection was finding a fraction of the duplicates on any corpus over 5000 items, which is every corpus this tool was built for. Three bugs compounding, none visible from a diff. The vec0 table is declared with no distance metric, so sqlite-vec returns L2. store.search filtered `1 - distance >= threshold`. That is not cosine at any scale: for unit vectors L2 is sqrt(2 - 2cos), so a 0.85 threshold silently meant cosine 0.989 and pruned every genuine near-duplicate below it. Nothing normalised vectors on write either, so the conversion was not valid to attempt in the first place - it worked only because ollama happens to return unit vectors, and would have been wrong per-model for a provider that does not. cluster.ts then routed 5000+ items through a candidate-limited path that passed the clustering threshold into that same filter and truncated the survivors to K=50. It was justified as an ANN trade-off, but sqlite-vec has no ANN index: vec0 KNN is a full scan with a LIMIT. So it bought no speed and cost recall. Measured on the 5285-item corpus that motivated the incident work: 26 clusters where exact pairwise finds 501, taking 20.6s against 9.3s. The path is deleted rather than tuned, because raising K while still passing a threshold into store.search reproduces the same bug at a larger limit and looks like a baseline. Vectors are normalised on write, similarity is derived, and a geometry version refuses to search a store written under the old representation instead of answering it wrongly. The check is on read, since the dangerous case is an old database opened by a new binary. Its backfill ships in this commit, not a later one: a version guard without a migration turns every existing database into one that refuses to search. `prism re-embed` also stamps it, because the guard's error message says it does. Separately, incident-closed PRs no longer rank as fully open. They now sit between open and closed. Promoting them outright was an overcorrection: the corpus is 993 open, 1347 merged, 2945 closed, so a ~900-item incident roughly doubled the tier the ranking exists to order. Every cluster count published before this is void as comparative evidence. The duplicates verified by hand against GitHub are still real; the model rankings built on these counts are not.
StressTestor
force-pushed
the
fix/cluster-geometry
branch
from
July 30, 2026 08:32
d27aa85 to
d2d8c40
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d27aa85951
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
|
|
||
| this.db.prepare("DELETE FROM vec_items WHERE id = ?").run(id); | ||
| const unit = toUnitVector(item.embedding); |
There was a problem hiding this comment.
When an existing database is upgraded and a normal scan embeds new or changed items, this write normalizes only those vectors while untouched legacy vectors remain raw and the store stays marked as geometry 0. findDuplicateClusters() bypasses assertVectorGeometry() and reads both representations directly; although pairwise cosine is scale-invariant, its refinement step computes an arithmetic centroid for components larger than three items, so raw vectors are weighted by their original magnitudes and cluster membership depends on which items happened to be refreshed. Require a backfill/re-embed before normalized incremental writes are used for clustering, or enforce the geometry guard on clustering reads.
Useful? React with 👍 / 👎.
Merged
StressTestor
added a commit
that referenced
this pull request
Jul 30, 2026
Rewrites the accumulated [unreleased] entries as one coherent 4.0.0 entry, led by an upgrading section rather than a feature list. The headline for anyone already running pr-prism is not incident-aware ranking, it is that their database refuses to search until it is converted. Corrects text that went stale while the entries accumulated: an entry pointing at "the vector-geometry fix below" when it sits above, star-map PR #11 described as still needing to land after it merged, server/triage.ts described as still hand-rolling metadata after #29 fixed it, and incident-closed PRs described as ranking "as open" after #30 moved them between open and closed. #19 and #27 are both closed. ARCHITECTURE described similarity.ts as "ANN pre-filtering, matryoshka truncation". It contains neither, only cosineSimilarity and isZeroVector. That was already wrong and the ANN path no longer exists at all. Not corrected, deliberately: the README's "594 duplicate clusters on 6K+ items" predates the candidate-limited path (added in v0.8), so it was measured under exact comparison and is accurate again now. The matryoshka benchmark ran on 2000 items, below the old threshold, so it never took the broken path either. Co-authored-by: StressTestor <StressTestor@users.noreply.github.com>
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
1 participant
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.
Duplicate detection was finding a fraction of the duplicates on any corpus over 5000 items, which is every corpus this tool was built for.
On the 5285-item odysseus corpus: 26 clusters found where exact pairwise finds 501. The path that produced 26 was also slower, 20.6s against 9.3s.
Three bugs compounding, none of them visible from a diff.
1. the similarity filter was on the wrong scale
vec0is declared with no distance metric, so sqlite-vec returns L2.store.searchfiltered1 - distance >= threshold.That is not cosine at any scale. For unit vectors L2 is
sqrt(2 - 2cos), so a 0.85 threshold silently meant cosine 0.989. Every genuine near-duplicate below that was pruned before anything else got a look at it. It is also why every cluster i inspected had a minSimilarity of 0.98 or above, which i noted as a curiosity at the time rather than as the bug's signature.2. nothing normalised on write
So the conversion was not valid to attempt in the first place. It appeared to work only because ollama returns unit vectors; for a provider that does not, the error would have been wrong in a model-dependent way, which is exactly the shape that corrupts a model comparison.
3. the candidate-limited path
cluster.tsrouted 5000+ items through a path that passed the clustering threshold into that wrong-scale filter, then truncated whatever survived to K=50.It was justified as an ANN trade-off. sqlite-vec has no ANN index:
vec0KNN is itself a full scan with aLIMIT(asg017/sqlite-vec#25). So it bought no speed and cost recall.Deleted rather than tuned. Raising K while still passing a threshold into
store.searchreproduces the same bug at a larger limit and looks like a baseline. 5285 x 768 floats is milliseconds.the migration
Stores written before this hold raw vectors.
search()refuses them with an actionable message instead of answering with confidently wrong similarities. The check is on read, because the dangerous case is an old database opened by a new binary, which no insert-time assertion can see.The backfill ships in this PR. A version guard without its migration turns every existing database into one that refuses to search.
prism re-embedalso stamps it, since the guard's error message says it does, and there is a test that fails if it stops being true.also: incident-closed ranking
Incident-closed PRs now rank between open and closed rather than as fully open.
An item closed by a repository-wide event never got a maintainer verdict, so it has to outrank a deliberate close. But it is not evidence of live work the way an open PR is. The corpus is 993 open, 1347 merged, 2945 closed: promoting a ~900-item incident to open-equivalent roughly doubled the tier the ranking exists to order.
what this voids
Every cluster count published before this is void as comparative evidence. The model benchmark numbers measured near-exact-duplicate detection, not duplicate detection at the configured threshold.
The duplicates verified by hand against GitHub are still real duplicates. What is dead is the claim that one model finds them and another does not. Re-benchmarking has to happen under corrected geometry before any model default moves.
514 tests, lint, typecheck and build clean. Closes #19.