From a4943e53bc103a4e8e818140cbc846aaacdb9ff3 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Wed, 24 Jun 2026 16:26:39 -0400 Subject: [PATCH] spike: ts_rank_cd ordering + non-vacuous relevance test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- crates/buzz-search/src/postgres.rs | 62 +++++++---- .../tests/e2e_nostr_interop.rs | 100 ++++++++++++++---- 2 files changed, 122 insertions(+), 40 deletions(-) diff --git a/crates/buzz-search/src/postgres.rs b/crates/buzz-search/src/postgres.rs index 00e6aa3cf..5fd73b2e9 100644 --- a/crates/buzz-search/src/postgres.rs +++ b/crates/buzz-search/src/postgres.rs @@ -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 Result Result Result("rank").unwrap_or(0.0) as f64 + } else { + 0.0 + }, }); } diff --git a/crates/buzz-test-client/tests/e2e_nostr_interop.rs b/crates/buzz-test-client/tests/e2e_nostr_interop.rs index b71cab6c5..36fce674c 100644 --- a/crates/buzz-test-client/tests/e2e_nostr_interop.rs +++ b/crates/buzz-test-client/tests/e2e_nostr_interop.rs @@ -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, +) -> 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 = 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::>() + ); + assert!( + returned_ids.contains(&id3), + "msg3 (separated terms) missing from results — query/index parity broken. \ + All results: {:?}", + events.iter().map(|e| &e.content).collect::>() + ); + + // 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::>() );