mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-07-22 07:48:27 +00:00
fix(store): order NIP-50 search by relevance (bm25), not created_at
NIP-50: results are returned "in descending order by quality of search result ... not by the usual .created_at", with the limit applied after the score. The store sorted search by created_at DESC (pre-existing), so it returned the newest matches rather than the best ones. makeSimpleSearch (the search [+ kinds/authors/since/until] + limit shape) now orders by FTS5 bm25 (ORDER BY event_fts.rank, created_at DESC as a tie-break). Verified bm25 rank works on the contentless table through the join, and that a stronger-but-older match outranks a weaker-but-newer one. The rarer search+specific-tag shape and the negentropy snapshot still sort by created_at (the row-id subquery can't carry rank; negentropy is a sync set) — documented. This is a correctness fix, not a scaling one: bm25 scores every match, so search latency still grows with the match set. Tests: SearchRelevanceOrderTest, Fts5CapabilityProbe.bm25RankWorksOnContentlessTableInAJoin; QueryAssemblerTest search plans updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XqGuBuSUsRudGerPqDuoKA
This commit is contained in:
@@ -9,13 +9,13 @@ flat. Two of those (author-timeline, follow-feed) were already addressed
|
||||
the report under the *client* `DefaultIndexingStrategy` rather than
|
||||
`relayIndexingStrategy()`. This change adds the **large-IN tag watcher** to the
|
||||
merge executor, fixes the **FTS delete path** (which degraded with corpus
|
||||
size) and shrinks the FTS index, and fixes a **statement-cache** miss the merge
|
||||
paths hit.
|
||||
size) and shrinks the FTS index, fixes **NIP-50 search ordering** (it was
|
||||
sorting by `created_at`, not relevance), and fixes a **statement-cache** miss
|
||||
the merge paths hit.
|
||||
|
||||
It does **not** fix NIP-50 *search* latency: that is bounded by the
|
||||
`created_at`-ordered sort over all matches, which FTS5 can't early-terminate
|
||||
while staying NIP-01-compliant (see §1). Corpus-independent search is an
|
||||
external-engine job.
|
||||
It does **not** fix NIP-50 *search* latency: bm25 relevance scoring (like the
|
||||
old `created_at` sort) must visit every match, so search cost still grows with
|
||||
the match set (§1). Corpus-independent search is an external-engine job.
|
||||
|
||||
Everything here is read/size work; the write path and on-disk index set are
|
||||
unchanged except the FTS table, which gets *smaller* and deletes faster.
|
||||
@@ -58,18 +58,28 @@ in-memory:
|
||||
By-column grows ~linearly with the table (O(n)/delete); by-rowid is flat —
|
||||
~78× at 8k rows and widening.
|
||||
|
||||
**Search is deliberately unchanged and still `created_at`-ordered.** NIP-01's
|
||||
`limit` requires the newest events *by `created_at`*, and FTS5 only
|
||||
early-terminates on its own rowid — so `MATCH … ORDER BY created_at DESC LIMIT
|
||||
n` still materializes and sorts every match, and search latency still grows
|
||||
with the match set (the report's 18× curve). An earlier draft added a
|
||||
`searchOrderByRowId` flag that ordered by the FTS rowid to get O(limit) search;
|
||||
that is *ingestion* order, which returns the wrong events under a limit once
|
||||
ingestion diverges from `created_at` (any historical sync) — a NIP-01
|
||||
violation — so it was removed. `optimize` compacts the index but does not
|
||||
change the asymptotics (measured within noise at 100k/200k). Corpus-independent
|
||||
search is genuinely an external-engine job (the Vespa side of the report), not
|
||||
this index.
|
||||
**Search ordering fixed to relevance (NIP-50), which the store was getting
|
||||
wrong.** NIP-50 says results are returned "in descending order by quality of
|
||||
search result ... not by the usual `.created_at`", with the limit applied after
|
||||
the score — but the store sorted search by `created_at DESC` (pre-existing).
|
||||
`makeSimpleSearch` (the `search [+ kinds/authors/since/until] + limit` shape)
|
||||
now orders by FTS5 bm25 (`ORDER BY event_fts.rank`, `created_at DESC` as a
|
||||
tie-break), verified against a stronger-but-older match outranking a
|
||||
weaker-but-newer one (`SearchRelevanceOrderTest`, `Fts5CapabilityProbe`). The
|
||||
rarer `search + specific tag` shape and the negentropy snapshot still sort by
|
||||
`created_at` (the row-id subquery can't carry rank through; negentropy is a
|
||||
sync set, not a ranked result) — a documented follow-up.
|
||||
|
||||
An earlier draft of *this* change instead added a `searchOrderByRowId` flag
|
||||
(order by the FTS rowid for O(limit) search); that is *ingestion* order — wrong
|
||||
events under a limit once ingestion diverges from time order, and not relevance
|
||||
either — so it was removed.
|
||||
|
||||
This is a **correctness** fix, not a scaling one: bm25 (like the created_at
|
||||
sort) must score every match, so search latency still grows with the match set
|
||||
(the report's 18× curve) and `optimize` doesn't change the asymptotics
|
||||
(measured within noise at 100k/200k). Corpus-independent search is genuinely an
|
||||
external-engine job (the Vespa side of the report), not this index.
|
||||
|
||||
Migration (v4→v5): the old rowids can't be remapped, so `event_fts` is dropped
|
||||
and rebuilt — synchronous stores rebuild in the upgrade transaction (client
|
||||
|
||||
@@ -49,12 +49,16 @@ import com.vitorpamplona.quartz.utils.EventFactory
|
||||
* external-content — which reads the source column from the base table —
|
||||
* cannot express it; contentless is the correct primitive.
|
||||
*
|
||||
* Search still orders by `event_headers.created_at DESC` (NIP-01 `limit`
|
||||
* semantics: the newest events *by created_at*), joining back to
|
||||
* `event_headers` on `row_id = event_fts.rowid`. FTS5 cannot early-terminate
|
||||
* that — it materializes and sorts all matches — so search latency still
|
||||
* grows with the match set; corpus-independent search needs an external
|
||||
* engine, not this index.
|
||||
* Search results are ordered by **relevance**, per NIP-50 ("descending order
|
||||
* by quality of search result ... not by the usual `.created_at`", limit
|
||||
* applied after the score) — via FTS5 bm25 (`ORDER BY event_fts.rank`), with
|
||||
* `created_at DESC` only as a tie-break. This is [QueryBuilder.makeSimpleSearch]
|
||||
* (the `search [+ kinds/authors/since/until] + limit` shape); the rarer
|
||||
* `search + specific tag` combination still sorts by `created_at` (its row-id
|
||||
* subquery can't carry the rank through), and the negentropy snapshot keeps
|
||||
* `created_at` (a sync set, not a ranked result). bm25 must score every match,
|
||||
* so search latency still grows with the match set regardless of ordering;
|
||||
* corpus-independent search needs an external engine, not this index.
|
||||
*
|
||||
* When [enabled] is `false` the module becomes an inert no-op: no
|
||||
* `event_fts` virtual table and no `fts_foreign_key` delete trigger are
|
||||
|
||||
@@ -1005,7 +1005,13 @@ class QueryBuilder(
|
||||
if (clause.conditions.isNotEmpty()) {
|
||||
append("\nWHERE ${clause.conditions}")
|
||||
}
|
||||
append("\nORDER BY event_headers.created_at DESC")
|
||||
// NIP-50: search results are ordered by relevance ("quality of
|
||||
// search result"), not created_at, and the limit is applied
|
||||
// after the score. FTS5 exposes bm25 as the `rank` column (more
|
||||
// negative = more relevant), so ORDER BY rank ascending is
|
||||
// best-match-first. created_at DESC is only a tie-break so
|
||||
// equally-relevant matches come newest-first deterministically.
|
||||
append("\nORDER BY ${fts.tableName}.rank, event_headers.created_at DESC")
|
||||
if (indexStrategy.useAndIndexIdOnOrderBy) {
|
||||
append(", event_headers.id ASC")
|
||||
}
|
||||
|
||||
@@ -714,7 +714,7 @@ class QueryAssemblerTest : BaseDBTest() {
|
||||
SELECT event_headers.id, event_headers.pubkey, event_headers.created_at, event_headers.kind, event_headers.tags, event_headers.content, event_headers.sig FROM event_headers
|
||||
INNER JOIN event_fts ON event_headers.row_id = event_fts.rowid
|
||||
WHERE (event_fts MATCH "keywords") AND (event_headers.pubkey IN ("7c5eb72a4584fdaaeaa145b25c92ea9917704224951219dbd43acef9e91fb88d", "f3ac434d61bc0f491a814782ccfdf9c439dae1f0bde9097ad4a245f4c495cd14", "12ae0fd81c85e1e7d9ed096397dc3129849425fe6f8afce7213ebf38ddfc6ca9"))
|
||||
ORDER BY event_headers.created_at DESC, event_headers.id ASC
|
||||
ORDER BY event_fts.rank, event_headers.created_at DESC, event_headers.id ASC
|
||||
├── SCAN event_fts VIRTUAL TABLE INDEX 0:M1
|
||||
├── SEARCH event_headers USING INTEGER PRIMARY KEY (rowid=?)
|
||||
└── USE TEMP B-TREE FOR ORDER BY
|
||||
@@ -727,7 +727,7 @@ class QueryAssemblerTest : BaseDBTest() {
|
||||
SELECT event_headers.id, event_headers.pubkey, event_headers.created_at, event_headers.kind, event_headers.tags, event_headers.content, event_headers.sig FROM event_headers
|
||||
INNER JOIN event_fts ON event_headers.row_id = event_fts.rowid
|
||||
WHERE (event_fts MATCH "keywords") AND (event_headers.pubkey IN ("7c5eb72a4584fdaaeaa145b25c92ea9917704224951219dbd43acef9e91fb88d", "f3ac434d61bc0f491a814782ccfdf9c439dae1f0bde9097ad4a245f4c495cd14", "12ae0fd81c85e1e7d9ed096397dc3129849425fe6f8afce7213ebf38ddfc6ca9"))
|
||||
ORDER BY event_headers.created_at DESC
|
||||
ORDER BY event_fts.rank, event_headers.created_at DESC
|
||||
├── SCAN event_fts VIRTUAL TABLE INDEX 0:M1
|
||||
├── SEARCH event_headers USING INTEGER PRIMARY KEY (rowid=?)
|
||||
└── USE TEMP B-TREE FOR ORDER BY
|
||||
@@ -741,7 +741,7 @@ class QueryAssemblerTest : BaseDBTest() {
|
||||
fun testKindAndSearch() =
|
||||
forEachDB { db ->
|
||||
val filter = Filter(kinds = listOf(1, 1111, 10000), search = "keywords")
|
||||
val orderBy = if (db.indexStrategy.useAndIndexIdOnOrderBy) "event_headers.created_at DESC, event_headers.id ASC" else "event_headers.created_at DESC"
|
||||
val orderBy = if (db.indexStrategy.useAndIndexIdOnOrderBy) "event_fts.rank, event_headers.created_at DESC, event_headers.id ASC" else "event_fts.rank, event_headers.created_at DESC"
|
||||
assertEquals(
|
||||
"""
|
||||
SELECT event_headers.id, event_headers.pubkey, event_headers.created_at, event_headers.kind, event_headers.tags, event_headers.content, event_headers.sig FROM event_headers
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/*
|
||||
* Copyright (c) 2025 Vitor Pamplona
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person obtaining a copy of
|
||||
* this software and associated documentation files (the "Software"), to deal in
|
||||
* the Software without restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
|
||||
* Software, and to permit persons to whom the Software is furnished to do so,
|
||||
* subject to the following conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be included in all
|
||||
* copies or substantial portions of the Software.
|
||||
*
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
|
||||
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
|
||||
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
|
||||
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
|
||||
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
|
||||
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
|
||||
*/
|
||||
package com.vitorpamplona.quartz.nip01Core.store.sqlite
|
||||
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync
|
||||
import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
|
||||
/**
|
||||
* NIP-50: search results are ordered by "quality of search result" (relevance),
|
||||
* **not** by `created_at`, and the limit is applied after the score. The store
|
||||
* uses FTS5 bm25 (`ORDER BY event_fts.rank`), so a stronger match outranks a
|
||||
* newer one.
|
||||
*/
|
||||
class SearchRelevanceOrderTest {
|
||||
private val signer = NostrSignerSync()
|
||||
|
||||
private fun note(
|
||||
content: String,
|
||||
createdAt: Long,
|
||||
) = signer.sign(TextNoteEvent.build(content, createdAt = createdAt))
|
||||
|
||||
@Test
|
||||
fun strongerMatchOutranksNewer() =
|
||||
runBlocking {
|
||||
val store = EventStore(dbName = null)
|
||||
try {
|
||||
// Older, but the term appears 3× in a short doc → most relevant.
|
||||
val strong = note("bitcoin bitcoin bitcoin", createdAt = 1_000)
|
||||
// Newer, term once buried in a long doc → least relevant.
|
||||
val weak = note("bitcoin is one topic among many other unrelated words here padding", createdAt = 9_000)
|
||||
// Newer still, but does not match at all.
|
||||
val nonMatch = note("completely different subject entirely", createdAt = 9_999)
|
||||
|
||||
store.insert(weak)
|
||||
store.insert(strong)
|
||||
store.insert(nonMatch)
|
||||
|
||||
val results = store.query<Event>(Filter(search = "bitcoin", limit = 10)).map { it.id }
|
||||
// Relevance order (strong before weak) — the opposite of
|
||||
// created_at DESC (which would put weak first) — and the
|
||||
// non-matching note is absent.
|
||||
assertEquals(listOf(strong.id, weak.id), results)
|
||||
} finally {
|
||||
store.close()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun limitAppliesAfterRelevanceScore() =
|
||||
runBlocking {
|
||||
val store = EventStore(dbName = null)
|
||||
try {
|
||||
// Three docs of decreasing relevance but increasing created_at,
|
||||
// so created_at order and relevance order are exact opposites.
|
||||
val best = note("apple apple apple apple", createdAt = 1)
|
||||
val mid = note("apple apple filler words", createdAt = 2)
|
||||
val worst = note("apple among lots of other unrelated filler words here", createdAt = 3)
|
||||
store.insert(best)
|
||||
store.insert(mid)
|
||||
store.insert(worst)
|
||||
|
||||
// limit=2 after scoring keeps the two MOST RELEVANT, not the
|
||||
// two newest.
|
||||
val top2 = store.query<Event>(Filter(search = "apple", limit = 2)).map { it.id }
|
||||
assertEquals(listOf(best.id, mid.id), top2)
|
||||
} finally {
|
||||
store.close()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -40,13 +40,12 @@ import kotlin.test.Test
|
||||
* contentless schema keys deletes off the rowid (= event_headers.row_id),
|
||||
* an O(log n) primary-key seek. Every event removal fires this trigger
|
||||
* (replaceable rotation, kind-5, expiration, right-to-vanish).
|
||||
* 2. **Search scaling — the limit.** `MATCH … ORDER BY created_at DESC
|
||||
* LIMIT n` must materialize + sort *every* matching document (FTS5 only
|
||||
* early-terminates on its own rowid, and NIP-01's `limit` requires newest
|
||||
* *by created_at*), so search cost grows with the match set. Segment
|
||||
* `optimize` compacts the index but does not change that; corpus-
|
||||
* independent search needs an external engine. Shown fragmented vs
|
||||
* optimized to size the (secondary) compaction effect.
|
||||
* 2. **Search scaling — the limit.** `MATCH … ORDER BY rank LIMIT n` (NIP-50
|
||||
* relevance ordering, bm25) must score *every* matching document, so
|
||||
* search cost grows with the match set regardless of ordering (created_at
|
||||
* has the same shape). Segment `optimize` compacts the index but does not
|
||||
* change that; corpus-independent search needs an external engine. Shown
|
||||
* fragmented vs optimized to size the (secondary) compaction effect.
|
||||
*
|
||||
* Size search with `-DftsBenchScale=N` (default 1). Not an assertion test.
|
||||
*/
|
||||
|
||||
@@ -31,10 +31,11 @@ import kotlin.test.assertEquals
|
||||
* loudly instead of the store silently breaking search on delete.
|
||||
*
|
||||
* - `content=''` **contentless** table with `contentless_delete=1`: lets the
|
||||
* index drop the duplicated content column yet still delete rows (the
|
||||
* `fts_foreign_key` trigger needs it).
|
||||
* - explicit `rowid` on insert + `ORDER BY rowid DESC LIMIT n`: the
|
||||
* early-terminating recency search path.
|
||||
* index drop the duplicated content column yet still delete rows by rowid
|
||||
* (the `fts_foreign_key` trigger needs it).
|
||||
* - explicit `rowid` on insert (= `event_headers.row_id`): the join key and
|
||||
* the O(log n) delete key.
|
||||
* - bm25 `rank` on a contentless table, through a join: NIP-50 relevance order.
|
||||
* - `'merge'` / `'optimize'` maintenance commands: segment compaction.
|
||||
*/
|
||||
class Fts5CapabilityProbe {
|
||||
@@ -64,6 +65,40 @@ class Fts5CapabilityProbe {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun bm25RankWorksOnContentlessTableInAJoin() {
|
||||
// NIP-50 orders by relevance, not created_at. Verify FTS5 bm25 `rank`
|
||||
// works on a contentless table and is reachable through the same
|
||||
// join-back-to-base-table shape the store's search query uses.
|
||||
val db = BundledSQLiteDriver().open(":memory:")
|
||||
try {
|
||||
db.execSQL("CREATE TABLE headers (row_id INTEGER PRIMARY KEY, created_at INTEGER, tag TEXT)")
|
||||
db.execSQL("CREATE VIRTUAL TABLE fts USING fts5(content, content='', contentless_delete=1)")
|
||||
// row 10: term appears 3× in a short doc (most relevant).
|
||||
// row 20: term once in a long doc (least relevant) but NEWER.
|
||||
db.execSQL("INSERT INTO headers VALUES (10, 100, 'A')")
|
||||
db.execSQL("INSERT INTO fts(rowid, content) VALUES (10, 'needle needle needle')")
|
||||
db.execSQL("INSERT INTO headers VALUES (20, 999, 'B')")
|
||||
db.execSQL("INSERT INTO fts(rowid, content) VALUES (20, 'needle alpha beta gamma delta epsilon zeta eta')")
|
||||
|
||||
// created_at DESC would return B (999) first; relevance returns A.
|
||||
val byRank = ArrayList<String>()
|
||||
db
|
||||
.prepare(
|
||||
"""
|
||||
SELECT headers.tag FROM headers
|
||||
INNER JOIN fts ON headers.row_id = fts.rowid
|
||||
WHERE fts MATCH 'needle'
|
||||
ORDER BY fts.rank
|
||||
LIMIT 10
|
||||
""".trimIndent(),
|
||||
).use { while (it.step()) byRank.add(it.getText(0)) }
|
||||
assertEquals(listOf("A", "B"), byRank, "bm25 rank must put the more relevant (shorter, higher-tf) doc first")
|
||||
} finally {
|
||||
db.close()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun segmentMergeAndOptimizeAreSupported() {
|
||||
val db = BundledSQLiteDriver().open(":memory:")
|
||||
|
||||
Reference in New Issue
Block a user