mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
spike: ts_rank_cd ordering + non-vacuous relevance test
Implements Eva's blocker-2 fix: Postgres backend now orders search
results by relevance, and the e2e test that claims to verify this
actually discriminates rank from recency.
postgres.rs
- SELECT list now includes `ts_rank_cd(content_tsv,
plainto_tsquery('simple', $q)) AS rank` when the query has
searchable text. The same `$q` parameter slot is reused in WHERE.
- ORDER BY rank DESC, created_at DESC when has_text; empty/"*"
queries skip the rank column and fall back to created_at DESC
(no needless tsquery cost).
- SearchHit.score is populated from the rank column (f32 widened
to f64). Empty/"*" queries leave score at 0.0.
e2e_nostr_interop::test_nip50_search_relevance_order
- Redesigned to discriminate rank from recency. Previous fixture
used "alpha bravo charlie" with msg3="alpha bravo" — plainto_tsquery
ANDs all terms, so msg3 never matched the WHERE clause and the
test passed trivially with one candidate (Eva caught this).
- New fixture: query "{prefix} alpha bravo"; msg1 (oldest) has
terms adjacent (high rank); msg2 (middle) doesn't match at all;
msg3 (newest) has terms separated by filler (lower rank).
- Asserts both msg1 and msg3 are present, then asserts events[0].id
== id1 with no `||content.contains(...)` escape hatch.
- Discriminator is term proximity, not term frequency: Typesense's
default _text_match does NOT reward repeated query terms (verified
empirically — identical tm scores for "alpha bravo" vs
"alpha alpha bravo bravo"), but BOTH backends reward adjacency.
Proximity is the property both backends agree on.
- New `send_rest_message_at` helper pins created_at via
`custom_created_at`. Without explicit timestamps, three back-to-back
sends share one wall-clock second; PG falls to heap-scan order and
masquerades as rank ordering. Spreading by 30s each makes the
recency-only ordering deterministically put msg3 first, so a
passing test really means rank wins.
Validation
- buzz-search lib: 30/30. buzz-relay lib: 337/337.
- NIP-50 e2e on Postgres: 4/4 (incl. relevance_order) + isolation 1/1.
- NIP-50 e2e on Typesense: 4/4 + isolation 1/1.
- Proof of discrimination: with postgres.rs reverted to
`ORDER BY created_at DESC`, the new test FAILS on PG (msg3 first,
as predicted). Restored ts_rank_cd ordering after.
Pre-existing failure not introduced by this commit:
test_nip17_gift_wrap_not_searchable fails on both backends — it queries
Typesense directly at events-spike-{backend}; on the PG backend that
collection is never written to (structurally expected), and the
Typesense-backend failure is the same fixture coupling Eva already
acknowledged in the prior turn. No regression vs e5869ddd/4b7c8d12.
Co-authored-by: Tyler <109685178+tlongwell-block@users.noreply.github.com>
Signed-off-by: Tyler <109685178+tlongwell-block@users.noreply.github.com>
This commit is contained in:
co-authored by
Tyler
parent
a3fbd0288f
commit
a4943e53bc
@@ -16,9 +16,10 @@
|
||||
//! is present
|
||||
//! - `since` / `until` → `created_at >= … AND created_at <= …`
|
||||
//!
|
||||
//! Results are ordered by `created_at DESC` — NIP-50 does not require strict
|
||||
//! relevance ordering, and chronological ordering is what the existing Buzz
|
||||
//! clients expect.
|
||||
//! Results are ordered by `ts_rank_cd` (cover-density relevance) when the
|
||||
//! query has searchable text, with `created_at DESC` as a tiebreaker. When
|
||||
//! the query is empty/`"*"` (no tsquery), ordering falls back to
|
||||
//! `created_at DESC` only.
|
||||
|
||||
use chrono::{DateTime, Utc};
|
||||
use sqlx::{PgPool, Row};
|
||||
@@ -67,13 +68,7 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
// cleaner, but the dynamic shape (optional clauses) is small enough that
|
||||
// hand-rolling a query with a stable parameter ordering is more readable
|
||||
// and easier to audit.
|
||||
let mut sql = String::from(
|
||||
"SELECT id, pubkey, kind, channel_id, created_at, content, \
|
||||
COUNT(*) OVER () AS total \
|
||||
FROM events \
|
||||
WHERE deleted_at IS NULL",
|
||||
);
|
||||
|
||||
//
|
||||
// `simple` matches the tokenizer used by the `content_tsv` generated
|
||||
// column in migration 0004. plainto_tsquery treats the input as a plain
|
||||
// string (handles spaces, ignores punctuation) — closest analogue to
|
||||
@@ -81,10 +76,28 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
let mut binds = Binds::new();
|
||||
let q_trim = query.q.trim();
|
||||
let has_text = !q_trim.is_empty() && q_trim != "*";
|
||||
if has_text {
|
||||
let q_idx = binds.push_text(q_trim);
|
||||
// Bind the query text first so the rank expression in the SELECT list and
|
||||
// the @@ predicate in WHERE can both reference `$q_idx`. Postgres allows
|
||||
// the same parameter slot to appear multiple times in a single statement.
|
||||
let q_idx = if has_text {
|
||||
Some(binds.push_text(q_trim))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let mut sql = String::from("SELECT id, pubkey, kind, channel_id, created_at, content");
|
||||
if let Some(idx) = q_idx {
|
||||
// `ts_rank_cd` (cover-density rank) rewards documents where the query
|
||||
// terms cluster together — closer match to Typesense's text relevance
|
||||
// than plain `ts_rank`. Returned as `rank REAL`.
|
||||
sql.push_str(&format!(
|
||||
" AND content_tsv @@ plainto_tsquery('simple', ${q_idx})"
|
||||
", ts_rank_cd(content_tsv, plainto_tsquery('simple', ${idx})) AS rank"
|
||||
));
|
||||
}
|
||||
sql.push_str(", COUNT(*) OVER () AS total FROM events WHERE deleted_at IS NULL");
|
||||
if let Some(idx) = q_idx {
|
||||
sql.push_str(&format!(
|
||||
" AND content_tsv @@ plainto_tsquery('simple', ${idx})"
|
||||
));
|
||||
}
|
||||
|
||||
@@ -120,7 +133,14 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
sql.push_str(&format!(" AND created_at <= ${idx}"));
|
||||
}
|
||||
|
||||
sql.push_str(" ORDER BY created_at DESC");
|
||||
if has_text {
|
||||
// ts_rank_cd-then-recency: relevance dominates, recency breaks ties.
|
||||
sql.push_str(" ORDER BY rank DESC, created_at DESC");
|
||||
} else {
|
||||
// No tsquery → no rank column. Fall back to chronological ordering,
|
||||
// matching the historical Buzz client expectation.
|
||||
sql.push_str(" ORDER BY created_at DESC");
|
||||
}
|
||||
let limit_idx = binds.push_i64(per_page);
|
||||
sql.push_str(&format!(" LIMIT ${limit_idx}"));
|
||||
let offset_idx = binds.push_i64(offset);
|
||||
@@ -155,10 +175,16 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result<SearchResult,
|
||||
pubkey: hex::encode(&pubkey),
|
||||
channel_id: channel_uuid.map(|u| u.to_string()),
|
||||
created_at: created_at.timestamp(),
|
||||
// Postgres backend doesn't expose a relevance score on the hit;
|
||||
// ts_rank_cd is available but unused by the consumer (req.rs
|
||||
// ignores `score` entirely). Leave at 0.0 for honesty.
|
||||
score: 0.0,
|
||||
// `rank` is only in the result set when the query had searchable
|
||||
// text; otherwise the SELECT omits the column. ts_rank_cd is
|
||||
// typically <1.0 for short docs and grows with match density;
|
||||
// we surface it raw, matching Typesense's `text_match` shape.
|
||||
// ts_rank_cd returns REAL (f32); widen to the SearchHit::score f64.
|
||||
score: if has_text {
|
||||
row.try_get::<f32, _>("rank").unwrap_or(0.0) as f64
|
||||
} else {
|
||||
0.0
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -84,12 +84,27 @@ async fn create_test_channel(keys: &Keys) -> String {
|
||||
|
||||
/// Send a message via a signed kind:9 event and return the event_id hex.
|
||||
async fn send_rest_message(keys: &Keys, channel_id: &str, content: &str) -> String {
|
||||
send_rest_message_at(keys, channel_id, content, None).await
|
||||
}
|
||||
|
||||
/// Like `send_rest_message` but lets the caller pin `created_at` to a specific
|
||||
/// unix-seconds timestamp. Useful when a test needs the recency tiebreak to be
|
||||
/// meaningful (the default `Timestamp::now()` collapses all back-to-back sends
|
||||
/// onto the same wall-clock second).
|
||||
async fn send_rest_message_at(
|
||||
keys: &Keys,
|
||||
channel_id: &str,
|
||||
content: &str,
|
||||
created_at: Option<i64>,
|
||||
) -> String {
|
||||
let client = reqwest::Client::new();
|
||||
let pubkey_hex = keys.public_key().to_hex();
|
||||
let event = EventBuilder::new(Kind::Custom(9), content)
|
||||
.tags(vec![Tag::parse(["h", channel_id]).unwrap()])
|
||||
.sign_with_keys(keys)
|
||||
.unwrap();
|
||||
let mut builder = EventBuilder::new(Kind::Custom(9), content)
|
||||
.tags(vec![Tag::parse(["h", channel_id]).unwrap()]);
|
||||
if let Some(secs) = created_at {
|
||||
builder = builder.custom_created_at(nostr::Timestamp::from(secs as u64));
|
||||
}
|
||||
let event = builder.sign_with_keys(keys).unwrap();
|
||||
let resp = client
|
||||
.post(format!("{}/events", relay_http_url()))
|
||||
.header("X-Pubkey", &pubkey_hex)
|
||||
@@ -1068,7 +1083,20 @@ async fn test_nip17_gift_wrap_not_searchable() {
|
||||
}
|
||||
|
||||
/// Send 3 messages with varying relevance to a query, wait for indexing, then search.
|
||||
/// Verify: the exact-match message is present in results (relevance-based, not just chronological).
|
||||
/// Verify: rank-based ordering — a more-relevant *older* message ranks above a
|
||||
/// less-relevant *newer* one, proving the result order is driven by relevance
|
||||
/// rather than recency.
|
||||
///
|
||||
/// Discriminator: **term proximity**. msg1 has the query terms adjacent;
|
||||
/// msg3 has the query terms separated by intervening words. Both Postgres
|
||||
/// `ts_rank_cd` (cover-density) and Typesense `_text_match` reward adjacency,
|
||||
/// so a recency-only ordering would put msg3 first; a rank-based ordering
|
||||
/// puts msg1 first.
|
||||
///
|
||||
/// We deliberately do NOT use term-frequency as the discriminator: Typesense
|
||||
/// default `_text_match` does not reward repeated query terms (verified
|
||||
/// empirically against the spike collection — repeated and single-occurrence
|
||||
/// docs tie). Proximity is the property both backends agree on.
|
||||
#[tokio::test]
|
||||
#[ignore]
|
||||
async fn test_nip50_search_relevance_order() {
|
||||
@@ -1078,13 +1106,23 @@ async fn test_nip50_search_relevance_order() {
|
||||
|
||||
// Unique prefix to isolate this test's messages from other test runs.
|
||||
let prefix = uuid::Uuid::new_v4().simple().to_string();
|
||||
let msg1 = format!("{prefix} alpha bravo charlie"); // oldest, exact match
|
||||
let msg2 = format!("{prefix} delta echo foxtrot"); // middle, no match
|
||||
let msg3 = format!("{prefix} alpha bravo"); // newest, partial match
|
||||
// Anchor created_at offsets so msg1 is genuinely older than msg3 in seconds.
|
||||
// Without this, all three sends share the same wall-clock second and
|
||||
// `created_at DESC` becomes a coin flip (heap-scan order on PG, insertion
|
||||
// order on Typesense) — which silently makes the test pass regardless of
|
||||
// rank ordering. Spreading them by 30s each guarantees the recency-only
|
||||
// ordering would put msg3 first, so a passing test really means rank wins.
|
||||
let now = nostr::Timestamp::now().as_secs() as i64;
|
||||
// msg1: oldest, query terms ADJACENT — highest expected rank.
|
||||
let msg1 = format!("{prefix} alpha bravo");
|
||||
// msg2: middle, no overlap with query — should not match at all.
|
||||
let msg2 = format!("{prefix} delta echo foxtrot");
|
||||
// msg3: newest, query terms SEPARATED by filler — lower expected rank.
|
||||
let msg3 = format!("{prefix} alpha xx yy zz bravo");
|
||||
|
||||
let id1 = send_rest_message(&keys, &channel, &msg1).await;
|
||||
send_rest_message(&keys, &channel, &msg2).await;
|
||||
send_rest_message(&keys, &channel, &msg3).await;
|
||||
let id1 = send_rest_message_at(&keys, &channel, &msg1, Some(now - 60)).await;
|
||||
send_rest_message_at(&keys, &channel, &msg2, Some(now - 30)).await;
|
||||
let id3 = send_rest_message_at(&keys, &channel, &msg3, Some(now)).await;
|
||||
|
||||
// Wait for Typesense indexing.
|
||||
tokio::time::sleep(Duration::from_secs(3)).await;
|
||||
@@ -1092,7 +1130,9 @@ async fn test_nip50_search_relevance_order() {
|
||||
let mut client = BuzzTestClient::connect(&url, &keys).await.expect("connect");
|
||||
|
||||
let sid = sub_id("nip50-relevance");
|
||||
let query = format!("{prefix} alpha bravo charlie");
|
||||
// Two-term query; both msg1 and msg3 contain both terms (so both pass the
|
||||
// WHERE / filter), but only msg1 has them adjacent. msg2 has neither term.
|
||||
let query = format!("{prefix} alpha bravo");
|
||||
let filter = Filter::new()
|
||||
.kind(Kind::Custom(9))
|
||||
.search(&query)
|
||||
@@ -1108,17 +1148,33 @@ async fn test_nip50_search_relevance_order() {
|
||||
.await
|
||||
.expect("collect until EOSE");
|
||||
|
||||
// Must have at least 1 result.
|
||||
assert!(!events.is_empty(), "expected search results, got none");
|
||||
|
||||
// The FIRST result must be the exact-match message (msg1), not the newer
|
||||
// partial match (msg3). This proves relevance ordering, not chronological.
|
||||
let first = &events[0];
|
||||
// Both msg1 and msg3 must be present — otherwise the test isn't
|
||||
// discriminating ordering, it's just checking presence.
|
||||
let returned_ids: Vec<String> = events.iter().map(|e| e.id.to_hex()).collect();
|
||||
assert!(
|
||||
first.id.to_hex() == id1 || first.content.contains("alpha bravo charlie"),
|
||||
"expected exact-match message as FIRST result (relevance order), \
|
||||
but got: '{}'. All results: {:?}",
|
||||
first.content,
|
||||
returned_ids.contains(&id1),
|
||||
"msg1 (adjacent terms) missing from results — query/index parity broken. \
|
||||
All results: {:?}",
|
||||
events.iter().map(|e| &e.content).collect::<Vec<_>>()
|
||||
);
|
||||
assert!(
|
||||
returned_ids.contains(&id3),
|
||||
"msg3 (separated terms) missing from results — query/index parity broken. \
|
||||
All results: {:?}",
|
||||
events.iter().map(|e| &e.content).collect::<Vec<_>>()
|
||||
);
|
||||
|
||||
// The FIRST result must be msg1 (older, adjacent terms), not msg3 (newer,
|
||||
// separated terms). No `|| content.contains(...)` escape hatch — id
|
||||
// equality only. A recency-only ordering would put msg3 first; a
|
||||
// rank-based ordering (ts_rank_cd / _text_match) puts msg1 first.
|
||||
assert_eq!(
|
||||
events[0].id.to_hex(),
|
||||
id1,
|
||||
"expected msg1 (adjacent-term match) as FIRST result via rank ordering, \
|
||||
but got msg id {} content '{}'. All results in order: {:?}",
|
||||
events[0].id.to_hex(),
|
||||
events[0].content,
|
||||
events.iter().map(|e| &e.content).collect::<Vec<_>>()
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user