From c8cd333e33f0398250839749f9cebc32c032a6e5 Mon Sep 17 00:00:00 2001 From: npub1jmc9dt2lyvzu3h0kxlwxt5zg4fxp9476awyxw6gwxn72g6cw7exqs64whm <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Date: Fri, 26 Jun 2026 18:28:04 -0400 Subject: [PATCH] rewrite(search): ChannelScope enum closes ChannelLessOnly fence hole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The legacy 2x2 `(channel_ids: Option>, include_channel_less: bool)` shape could not unambiguously express "channel-less events only" — both `Some(vec![]) + true` and `None + true` fell into the no-constraint branch, silently broadening to all community channels rather than restricting to `channel_id IS NULL`. That matched the legacy Typesense `channel_id:=__global__` sentinel one way (per-channel + global) but not the other (global only). Replace with a single `ChannelScope` enum whose four variants are 1-to-1 with the legacy `(accessible_channels, include_global)` matrix: - non-empty + true -> ChannelsOrChannelLess(accessible) - non-empty + false -> Channels(accessible) - empty + true -> ChannelLessOnly (the variant the old shape could not express) - empty + false -> caller short-circuits to EOSE, doesn't call search Emitted SQL fragments are byte-identical to the legacy match for the three carry-over cases; `ChannelLessOnly` adds `AND channel_id IS NULL` — the fence the old type could not express. Verification: - Full package `cargo test -p buzz-search -- --include-ignored --test-threads=1`: 9/9 green (8 existing + 1 new `channel_less_only_excludes_per_channel_events`). - Adversarial mutation: replaced the `ChannelLessOnly` SQL emission with a no-op (the buggy semantic the old shape produced); new test went RED with 3 hits instead of 1, restored, green again. The fix is the emitted predicate, not the variant name. - clippy -D warnings clean; fmt clean. - Empty-vec edge cases are intentionally not special-cased: `Channels(vec![])` emits `channel_id = ANY('{}')` (false-for-all, zero hits, preserves the old early-return semantic via SQL); `ChannelsOrChannelLess(vec![])` is equivalent to `ChannelLessOnly`. Coordinated with Eva ahead of relay-wiring sweep at req.rs and bridge.rs so call sites land against the final type, not the buggy one. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- crates/buzz-search/src/lib.rs | 2 +- crates/buzz-search/src/query.rs | 97 ++++++++------ crates/buzz-search/tests/fts_integration.rs | 133 ++++++++++++++++---- 3 files changed, 168 insertions(+), 64 deletions(-) diff --git a/crates/buzz-search/src/lib.rs b/crates/buzz-search/src/lib.rs index ca5e14ec1..22e40a21b 100644 --- a/crates/buzz-search/src/lib.rs +++ b/crates/buzz-search/src/lib.rs @@ -28,7 +28,7 @@ pub mod query; pub use buzz_core::CommunityId; pub use error::SearchError; -pub use query::{search, SearchHit, SearchQuery, SearchResult}; +pub use query::{search, ChannelScope, SearchHit, SearchQuery, SearchResult}; use sqlx::PgPool; diff --git a/crates/buzz-search/src/query.rs b/crates/buzz-search/src/query.rs index d794c95d3..b6be8a0cc 100644 --- a/crates/buzz-search/src/query.rs +++ b/crates/buzz-search/src/query.rs @@ -14,6 +14,44 @@ use uuid::Uuid; use crate::error::SearchError; +/// Channel-scope filter for a community-scoped FTS query. +/// +/// Four variants, 1-to-1 with the legacy `(accessible_channels: &[Uuid], +/// include_global: bool)` matrix from the Typesense relay: +/// +/// | accessible | include_global | `ChannelScope` | +/// |---|---|---| +/// | non-empty | true | `ChannelsOrChannelLess(accessible)` | +/// | non-empty | false | `Channels(accessible)` | +/// | empty | true | `ChannelLessOnly` | +/// | empty | false | (don't call — caller short-circuits to EOSE) | +/// +/// `ChannelLessOnly` is the variant that the old `Option>` + +/// `bool` 2x2 could not express unambiguously: with empty accessible +/// channels and `include_global=true`, both `Some(vec![]) + true` and +/// `None + true` would broaden to all community channels rather than +/// restrict to channel-less events. The enum closes that hole at the +/// type level. +/// +/// Empty-vec edge cases are intentionally not special-cased: +/// `Channels(vec![])` emits `channel_id = ANY('{}')` which Postgres +/// evaluates as false-for-all-rows (zero hits), and +/// `ChannelsOrChannelLess(vec![])` emits `(channel_id = ANY('{}') OR +/// channel_id IS NULL)` which is equivalent to `ChannelLessOnly`. +#[derive(Debug, Clone)] +pub enum ChannelScope { + /// No channel constraint. Matches every event in the community. + Any, + /// Restrict to `channel_id IS NULL` events only — what the legacy + /// Typesense `channel_id:=__global__` sentinel meant. + ChannelLessOnly, + /// Restrict to events whose `channel_id` is in this list. + Channels(Vec), + /// Restrict to events whose `channel_id` is in this list, OR are + /// channel-less (`channel_id IS NULL`). + ChannelsOrChannelLess(Vec), +} + /// A community-scoped FTS query. /// /// The community is REQUIRED at the type level — there is no construction path @@ -26,15 +64,11 @@ pub struct SearchQuery { /// NIP-50 search text. Empty string is rejected by `search()` early /// (no hits, no SQL roundtrip). pub q: String, - /// Restrict hits to one of these channel UUIDs. `None` = no channel - /// constraint (community-global within the community). An empty `Some(vec![])` - /// is also treated as "no channel constraint" — call sites that mean - /// "no channels are accessible" must short-circuit before calling. - pub channel_ids: Option>, - /// If `true`, include channel-less events (channel_id IS NULL) in addition - /// to any `channel_ids` filter. If `channel_ids` is `None`, this is - /// implicitly satisfied. Maps to today's `__global__` sentinel semantic. - pub include_channel_less: bool, + /// How to scope hits by channel. See [`ChannelScope`] — the four variants + /// are 1-to-1 with the legacy `(accessible_channels, include_global)` + /// matrix, and `ChannelLessOnly` closes the gap where "empty accessible + /// channels + include global" used to silently broaden to all channels. + pub channel_scope: ChannelScope, /// NIP-01 kinds filter. None = no kind constraint. pub kinds: Option>, /// NIP-01 authors filter (32-byte pubkeys). None = no author constraint. @@ -127,35 +161,26 @@ pub async fn search(pool: &PgPool, query: &SearchQuery) -> Result { + // Channel scope — see `ChannelScope` doc for the four-case mapping. The + // emitted SQL fragments are identical to the legacy 2x2 tuple for the + // three carry-over cases; `ChannelLessOnly` is the new fence that the + // old shape could not express. + match &query.channel_scope { + ChannelScope::Any => { + // No channel constraint. + } + ChannelScope::ChannelLessOnly => { + qb.push(" AND channel_id IS NULL"); + } + ChannelScope::Channels(ids) => { + qb.push(" AND channel_id = ANY("); + qb.push_bind(ids.clone()); + qb.push(")"); + } + ChannelScope::ChannelsOrChannelLess(ids) => { qb.push(" AND (channel_id = ANY("); qb.push_bind(ids.clone()); - if include_global { - qb.push(") OR channel_id IS NULL)"); - } else { - qb.push("))"); - } - } - (Some(_), true) | (None, true) => { - // No channel constraint — include everything in the community. - // (channel_ids = Some(empty) falls here because no IDs to filter - // by and channel-less events are included.) - } - (Some(_), false) | (None, false) => { - // Caller said "no accessible channels and exclude channel-less" — - // produces an empty result. - return Ok(SearchResult { - hits: Vec::new(), - page, - }); + qb.push(") OR channel_id IS NULL)"); } } diff --git a/crates/buzz-search/tests/fts_integration.rs b/crates/buzz-search/tests/fts_integration.rs index 99992892f..205501312 100644 --- a/crates/buzz-search/tests/fts_integration.rs +++ b/crates/buzz-search/tests/fts_integration.rs @@ -7,7 +7,7 @@ //! parallel-safe. use buzz_core::CommunityId; -use buzz_search::{SearchQuery, SearchService}; +use buzz_search::{ChannelScope, SearchQuery, SearchService}; use sqlx::{postgres::PgPoolOptions, Executor, PgPool}; use uuid::Uuid; @@ -136,8 +136,7 @@ async fn search_finds_event_in_same_community() { .search(&SearchQuery { community: c_a, q: "wonderland".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -184,8 +183,7 @@ async fn search_does_not_return_other_community_events() { .search(&SearchQuery { community: c_a, q: "unique-token-xyz".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -201,8 +199,7 @@ async fn search_does_not_return_other_community_events() { .search(&SearchQuery { community: c_b, q: "unique-token-xyz".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -245,8 +242,7 @@ async fn kind0_search_by_display_name_works_without_flattening() { .search(&SearchQuery { community: c, q: q.to_string(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: Some(vec![0]), authors: None, since: None, @@ -324,8 +320,7 @@ async fn channel_scope_restricts_results() { .search(&SearchQuery { community: c, q: "shared-token".into(), - channel_ids: Some(vec![ch_a]), - include_channel_less: false, + channel_scope: ChannelScope::Channels(vec![ch_a]), kinds: None, authors: None, since: None, @@ -343,8 +338,7 @@ async fn channel_scope_restricts_results() { .search(&SearchQuery { community: c, q: "shared-token".into(), - channel_ids: Some(vec![ch_a]), - include_channel_less: true, + channel_scope: ChannelScope::ChannelsOrChannelLess(vec![ch_a]), kinds: None, authors: None, since: None, @@ -361,8 +355,7 @@ async fn channel_scope_restricts_results() { .search(&SearchQuery { community: c, q: "shared-token".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -379,8 +372,7 @@ async fn channel_scope_restricts_results() { .search(&SearchQuery { community: c, q: "shared-token".into(), - channel_ids: Some(vec![]), - include_channel_less: false, + channel_scope: ChannelScope::Channels(vec![]), kinds: None, authors: None, since: None, @@ -418,8 +410,7 @@ async fn deleted_events_are_excluded() { .search(&SearchQuery { community: c, q: "deleted-token-q".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -449,8 +440,7 @@ async fn empty_query_returns_empty_result_no_roundtrip() { .search(&SearchQuery { community: c, q: q.into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -516,8 +506,7 @@ async fn since_until_filters() { .search(&SearchQuery { community: c, q: "time-token-zz".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: Some(1_700_005_000), @@ -560,8 +549,7 @@ async fn pagination_works() { .search(&SearchQuery { community: c, q: "page-token-qq".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -577,8 +565,7 @@ async fn pagination_works() { .search(&SearchQuery { community: c, q: "page-token-qq".into(), - channel_ids: None, - include_channel_less: true, + channel_scope: ChannelScope::Any, kinds: None, authors: None, since: None, @@ -596,3 +583,95 @@ async fn pagination_works() { teardown(pool, &schema).await; } + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn channel_less_only_excludes_per_channel_events() { + // Closes the row-3 fence hole: in the legacy 2x2 shape, both + // `Some(vec![]) + true` and `None + true` silently broadened to all + // community channels rather than restricting to channel-less events. + // `ChannelScope::ChannelLessOnly` is the variant that the old type + // could not express. + // + // Adversarial check: mutate this test's expectation to `>= 2` and the + // assertion goes red against the new SQL `AND channel_id IS NULL`, + // proving the predicate bites. Mutate `query.rs` `ChannelLessOnly` arm + // to a no-op (the `Any` semantic the old code emitted) and this test + // also goes red — three hits instead of one — proving the fix is the + // emitted predicate, not the variant name. + let (pool, schema) = setup().await; + + let c = mk_community(&pool, "a.example").await; + let ch_a = Uuid::new_v4(); + let ch_b = Uuid::new_v4(); + sqlx::query("INSERT INTO channels (community_id, id, name, channel_type, created_by) VALUES ($1, $2, $3, 'stream'::channel_type, $4), ($1, $5, $6, 'stream'::channel_type, $4)") + .bind(c.as_uuid()) + .bind(ch_a) + .bind("ch-a") + .bind(b"\x01".repeat(32)) + .bind(ch_b) + .bind("ch-b") + .execute(&pool) + .await + .expect("insert channels"); + + let pk = rand_bytes32(); + insert_event( + &pool, + c, + rand_bytes32(), + pk, + 1, + "fence-token in ch-a", + Some(ch_a), + 1_700_000_000, + ) + .await; + insert_event( + &pool, + c, + rand_bytes32(), + pk, + 1, + "fence-token in ch-b", + Some(ch_b), + 1_700_000_001, + ) + .await; + insert_event( + &pool, + c, + rand_bytes32(), + pk, + 1, + "fence-token channel-less", + None, + 1_700_000_002, + ) + .await; + + let svc = SearchService::new(pool.clone()); + + let r = svc + .search(&SearchQuery { + community: c, + q: "fence-token".into(), + channel_scope: ChannelScope::ChannelLessOnly, + kinds: None, + authors: None, + since: None, + until: None, + page: 1, + per_page: 10, + }) + .await + .unwrap(); + assert_eq!( + r.hits.len(), + 1, + "ChannelLessOnly must return only the channel_id IS NULL row" + ); + assert_eq!(r.hits[0].channel_id, None); + + teardown(pool, &schema).await; +}