From 54c8cefe6906f0893174bae7c72d0ff19e766fac Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 19:58:11 +0000 Subject: [PATCH] fix(quartz): don't time-walk a search filter in fetchAllPages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NIP-50 search results are ranked by relevance, not created_at, so paging a search filter by an `until` cursor silently degrades a top-N search into a full time-walk of the corpus — and never terminates against a relay that runs FTS over its whole corpus regardless of `until`. fetchAllPages now queries a `search` filter on its first page only: it is dropped from every later page and its hits neither advance nor drag back the `until` cursor that co-resident non-search filters page with. onNewPage also moves below the empty-page break so it never announces a page that isn't fetched. Adds a test proving a search filter returns a single relay page while a plain filter over the same capped relay still pages through the set. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015YEbdqCRPkszkGCoi89RMt --- .../NostrClientFetchAllPagesExt.kt | 37 +++++++++-- .../NostrClientReqBypassingRelayLimitsTest.kt | 64 +++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/NostrClientFetchAllPagesExt.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/NostrClientFetchAllPagesExt.kt index 9c09bf586d..8015467c41 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/NostrClientFetchAllPagesExt.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/relay/client/accessories/NostrClientFetchAllPagesExt.kt @@ -45,6 +45,15 @@ import kotlin.math.min * stops when all filters with limits are fulfilled or when a page returns no events. * Filters without a limit are considered unbounded and only stop on empty pages. * + * A `search` ([Filter.search]) filter is the exception: NIP-50 results are ranked by + * relevance, not `created_at`, so paging one by a `until` cursor is meaningless — it + * would silently turn a top-N search into a time-walk, and never terminate against a + * relay that runs FTS over its whole corpus regardless of `until`. So a search filter + * is queried on the FIRST page only; it is then dropped from every later page and its + * hits never advance (nor drag back) the `until` cursor other filters page with. Give + * it a `limit` to bound that single page; without one you get the relay's default page + * of top hits. + * * @param relay The relay to query. * @param filters Filters to apply on every page (the `until` field is overwritten per page). * @param timeoutMs Maximum time to wait for a single page's EOSE before giving up. @@ -86,21 +95,30 @@ suspend fun INostrClient.fetchAllPages( if (until == null) { filters } else { - onNewPage?.invoke(until) filters.map { it.copy(until = until) } } - // Only include filters that still need more events. + // Only include filters that still need more events. A `search` filter is + // relevance-ranked, not `created_at`-ordered, so it is queried on the first + // page only (until == null) and dropped from every later page — paging it by + // `until` would corrupt its ordering and can never terminate. val remainingFilters = pagedFilters.filterIndexed { index, filter -> val limit = filter.limit - limit == null || matchCountPerFilter[index] < limit + val stillNeedsMore = limit == null || matchCountPerFilter[index] < limit + val pageable = until == null || filter.search == null + stillNeedsMore && pageable } if (remainingFilters.isEmpty()) break + // Announce the page only now that we know it will actually be fetched: a + // search-only filter drops out of remainingFilters above and breaks with no + // REQ, so firing this earlier would report a page that never happens. + if (until != null) onNewPage?.invoke(until) + val doneChannel = Channel(Channel.CONFLATED) var pageCount = 0 @@ -117,17 +135,24 @@ suspend fun INostrClient.fetchAllPages( ) { // Check if the relay is returning what we asked before moving forward var atLeastOne = false + // Only a paginating (non-search) filter may advance the `until` + // cursor. A search filter's hits — possibly old, relevance-ranked + // — must not drag the cursor back, or the next page would skip + // events a co-resident normal filter still needs. + var advancesCursor = false for (i in pagedFilters.indices) { - val limit = pagedFilters[i].limit - if ((limit == null || matchCountPerFilter[i] < limit) && pagedFilters[i].match(event)) { + val filter = pagedFilters[i] + val limit = filter.limit + if ((limit == null || matchCountPerFilter[i] < limit) && filter.match(event)) { matchCountPerFilter[i]++ atLeastOne = true + if (filter.search == null) advancesCursor = true } } if (atLeastOne) { onEvent(event) pageCount++ - if (event.createdAt < pageMinTs) { + if (advancesCursor && event.createdAt < pageMinTs) { pageMinTs = event.createdAt } } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NostrClientReqBypassingRelayLimitsTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NostrClientReqBypassingRelayLimitsTest.kt index dac066c916..3b4f476edb 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NostrClientReqBypassingRelayLimitsTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/nip01Core/relay/NostrClientReqBypassingRelayLimitsTest.kt @@ -20,17 +20,26 @@ */ package com.vitorpamplona.quartz.nip01Core.relay +import com.vitorpamplona.geode.InProcessRelays import com.vitorpamplona.geode.fixtures.SyntheticEvents import com.vitorpamplona.geode.testing.RelayClientTest import com.vitorpamplona.geode.testing.preload import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent +import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.fetchAllPages import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.LimitsPolicy +import com.vitorpamplona.quartz.nip01Core.relay.server.policies.RelayLimits import com.vitorpamplona.quartz.nip02FollowList.ContactListEvent +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertTrue class NostrClientReqBypassingRelayLimitsTest : RelayClientTest() { @Test @@ -110,4 +119,59 @@ class NostrClientReqBypassingRelayLimitsTest : RelayClientTest() { assertEquals(1000, metadataEvents.size) assertEquals(1500, contactListEvents.size) } + + /** + * A `search` filter is relevance-ranked, not `created_at`-ordered, so + * `fetchAllPages` must fetch only its FIRST page and never advance the + * `until` cursor — otherwise a NIP-50 top-N search silently degrades into a + * full time-walk of the corpus. A plain (non-search) filter over the same + * capped relay is the control: it *does* page through everything, proving + * the per-REQ cap is real and pagination is actually happening. + */ + @Test + fun searchFilterIsFetchedAsSingleRelevancePageNotTimeWalked() = + runBlocking { + // A relay that returns at most 2 events per REQ (defaultLimit fills in + // for a filter that gives no limit), so an unbounded filter must + // paginate to drain a larger set. + val cappedHub = InProcessRelays(defaultPolicy = { LimitsPolicy(RelayLimits(maxLimit = 2, defaultLimit = 2)) }) + val cappedScope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + val cappedClient = NostrClient(cappedHub, cappedScope) + try { + // Five kind-1 notes, distinct created_at (via idSeed), all matching + // the FTS term "kotlin". + cappedHub.getOrCreate(defaultRelayUrl).preload( + (1..5).map { SyntheticEvents.fakeEvent(idSeed = it, content = "kotlin note $it") }, + ) + + // Control: a non-search filter drains all five across pages and + // advances the cursor (onNewPage fires) — the cap is real. + val controlEvents = mutableListOf() + var controlPages = 0 + cappedClient.fetchAllPages( + relay = defaultRelayUrl, + filters = listOf(Filter(kinds = listOf(1))), + onNewPage = { controlPages++ }, + ) { controlEvents.add(it) } + assertEquals(5, controlEvents.size, "non-search filter must page through the whole set") + assertTrue(controlPages > 0, "non-search filter must advance the until cursor across pages") + + // Search filter: only the first relevance-ranked page is fetched. + val searchEvents = mutableListOf() + var searchPages = 0 + val searchTotal = + cappedClient.fetchAllPages( + relay = defaultRelayUrl, + filters = listOf(Filter(search = "kotlin")), + onNewPage = { searchPages++ }, + ) { searchEvents.add(it) } + assertEquals(2, searchTotal, "a search filter must be fetched as a single page (the relay's cap)") + assertEquals(2, searchEvents.size) + assertEquals(0, searchPages, "a search filter must never advance the until cursor") + } finally { + cappedClient.disconnect() + cappedScope.cancel() + cappedHub.close() + } + } }