mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-09 08:04:45 +00:00
fix(quartz): don't time-walk a search filter in fetchAllPages
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YEbdqCRPkszkGCoi89RMt
This commit is contained in:
+31
-6
@@ -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<Unit>(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
|
||||
}
|
||||
}
|
||||
|
||||
+64
@@ -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<Event>()
|
||||
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<Event>()
|
||||
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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user