mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(search): route only id-shaped text to the legacy scan, not every empty result
The first cut fell back to `findNotesStartingWith` whenever the filter path came up empty, so every zero-result keystroke scanned the whole cache twice — while typing, which is exactly when it is worst. The fallback exists for one reason: an id matches on `idHex`, which is not content and so nothing a filter's `search` can reach. So only text that could name an event takes it — a bech32 pointer, or a run of at least eight hex characters. An ordinary query now scans once. Also records the outcome in the plan: what shipped, the two things that changed on contact with the code (step 7 became moot, so `FilterMatcher` stays untouched and its blast radius never opens), and what is deliberately left — the user and channel finders, which are name-prefix lookups rather than event filters and would be worse expressed as one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017yKjw2WqwZpSzsqcYZMnkV
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
# Local search as `filter(Filter)` — retiring the bespoke `find*StartingWith` scans
|
||||
|
||||
_Status: proposal. Three decisions (§2) are open and change what the code looks like._
|
||||
_Status: **steps 1–6 shipped**; see §8 for what landed, what changed on contact with the code, and
|
||||
what is deliberately left. The three decisions in §2 were taken as recommended._
|
||||
|
||||
## 0. The shape of the thing
|
||||
|
||||
@@ -279,3 +280,51 @@ Call sites to migrate: `SearchBarViewModel`, `UserSuggestionState`, `UserSearchE
|
||||
mandatory-maintenance rule for §4.4.
|
||||
- `2026-09-07` search-field work (`SearchTokenizer` / `SearchFilterBuilder` in `commons/search/`)
|
||||
— produces the `Filter`s step 6 consumes.
|
||||
|
||||
|
||||
## 8. What actually shipped
|
||||
|
||||
Steps 1–6 landed. Two things changed on contact with the code, both for the better:
|
||||
|
||||
**Step 7 is moot, and the blast radius never opens.** The plan assumed local search needed
|
||||
`FilterMatcher` to honour `search`, which would have narrowed every filter carrying one — 33 feed
|
||||
filters, `FilterIndex`, geode's `MirrorWorker` — and needed an audit before flipping. It does not.
|
||||
`LocalCache.filter` grew a **predicate** parameter instead, and search composes an
|
||||
`EventSearchMatcher` into it. `FilterMatcher` is untouched, so nothing else can change behaviour.
|
||||
The predicate is also where viewer policy went (§2.2), so one parameter answers both.
|
||||
|
||||
**Step 1 turned out to be a pure win with no search in it.** `FilterMatcher` was allocating three
|
||||
ways per event; fixing that needed no new API and pays for all 33 existing callers. It is pinned by
|
||||
a differential test that keeps the old implementation as an oracle and fuzzes 20,000 random
|
||||
event/filter pairs against it.
|
||||
|
||||
**Step 2 is scoped, not universal.** `forEachIndexableField` has a default that falls back to
|
||||
`indexableContent()` — already free for the ~28 kinds whose indexable content is `content` itself —
|
||||
and only the kinds local search actually scans override it (text notes, long-form, wiki,
|
||||
highlights, classifieds, live activities, community definitions). The remaining ~79 joining
|
||||
implementations still allocate on the read path, which costs nothing until something scans them.
|
||||
A test pins every override's fields to rejoin to `indexableContent()` byte-for-byte, so the
|
||||
externally-mirrored kind table stays valid and no reindex is needed.
|
||||
|
||||
### Deliberately not done
|
||||
|
||||
- **`findUsersStartingWith` and the three channel finders stay.** They are name-prefix lookups over
|
||||
users and channels, not event filters; forcing them through a `Filter` would be worse, not
|
||||
better. The plan's framing ("retire the bespoke scans") was too broad — only the *note* search is
|
||||
filter-shaped.
|
||||
- **`findNotesStartingWith` stays, on one narrow path.** It matches `idHex.startsWith(text)` and
|
||||
resolves bech32 pointers, neither of which is content and so neither reachable through a
|
||||
filter's `search`. Text that could name an event — a bech32 pointer, or ≥8 hex characters — is
|
||||
routed to it; everything else goes through the filters. The first cut fell back to it whenever
|
||||
the filter path came up empty, which made every zero-result keystroke scan the cache twice.
|
||||
- **No characterization test for `findNotesStartingWith`.** The parity harness §6 asked for was not
|
||||
written; the id path it guards is now the only thing still using that scan, and it is unchanged.
|
||||
Worth writing if that path is ever touched.
|
||||
|
||||
### Semantics that changed
|
||||
|
||||
Local text matching is now **terms ANDed**, each a case-insensitive substring, where it was one
|
||||
literal phrase. A single-word query — the common case — is identical; `bitcoin lightning` now finds
|
||||
notes carrying both words rather than only that exact phrase, which is what the relay already
|
||||
returned for the same string. Ordering stays recency (§2.3): no local relevance score exists, and
|
||||
`EventSearchMatcher` answers only yes or no.
|
||||
|
||||
@@ -5,12 +5,12 @@ _Audited 2026-06-30. 21 plans: 19 shipped (archived), 1 in-progress, 1 queued, 0
|
||||
## In progress
|
||||
| Plan | Summary |
|
||||
| ---- | ------- |
|
||||
| [2026-09-07-generic-local-filter.md](2026-09-07-generic-local-filter.md) | Local note search moved onto `LocalCache.filter(Filter)` so the search box's tokens narrow local and relay results alike; steps 1–6 shipped, `FilterMatcher` left untouched via a predicate parameter. |
|
||||
| [2026-05-24-ios-support.md](2026-05-24-ios-support.md) | Incremental KMP-to-iOS port; quartz/commons iOS targets are configured (Phase 1) but no `iosApp` module exists yet. |
|
||||
|
||||
## Queued
|
||||
| Plan | Summary |
|
||||
| ---- | ------- |
|
||||
| [2026-09-07-generic-local-filter.md](2026-09-07-generic-local-filter.md) | Retire `CacheSearch`'s five bespoke scans in favour of `LocalCache.filter(Filter)` — needs `search` in `FilterMatcher`, an allocation-free indexable-field visitor, and viewer policy moved out to a predicate. Three decisions open. |
|
||||
| [2026-07-23-push-notification-redesign.md](2026-07-23-push-notification-redesign.md) | Per-kind tray notification redesign — accent colors, status-bar icons, MessagingStyle/BigPictureStyle/colorized zap cards, aggregation, Conversations/Bubbles; closes nutzap/onchain/repost/badge parity gaps. |
|
||||
| [2026-06-20-napplet-inter-applet.md](2026-06-20-napplet-inter-applet.md) | NAP-INC / NAP-INTENT inter-applet messaging — deferred; prerequisites (multi-applet hosting, archetype registry, `MESSAGING` capability) not yet built. |
|
||||
|
||||
|
||||
+24
-6
@@ -55,6 +55,7 @@ import com.vitorpamplona.quartz.nip05DnsIdentifiers.INip05Client
|
||||
import com.vitorpamplona.quartz.nip05DnsIdentifiers.Nip05Id
|
||||
import com.vitorpamplona.quartz.nip10Notes.content.findHashtags
|
||||
import com.vitorpamplona.quartz.nip19Bech32.Nip19Parser
|
||||
import com.vitorpamplona.quartz.nip19Bech32.decodeEventIdAsHexOrNull
|
||||
import com.vitorpamplona.quartz.nip19Bech32.entities.IPubKeyEntity
|
||||
import com.vitorpamplona.quartz.nip19Bech32.entities.NAddress
|
||||
import com.vitorpamplona.quartz.nip19Bech32.entities.NEvent
|
||||
@@ -300,12 +301,18 @@ class SearchBarViewModel(
|
||||
// path through findNotesStartingWith.
|
||||
val parsed = QueryParser.parse(term)
|
||||
val raw =
|
||||
if (parsed.isEmpty) {
|
||||
emptyList()
|
||||
} else {
|
||||
LocalCache.search
|
||||
.findNotesMatching(SearchFilterBuilder.build(parsed, limit = 200), account.hiddenUsers)
|
||||
.ifEmpty { LocalCache.search.findNotesStartingWith(term, account.hiddenUsers) }
|
||||
when {
|
||||
parsed.isEmpty -> emptyList()
|
||||
// An id, whole or half-typed, is a lookup rather than a search: it matches on
|
||||
// `idHex`, which is not content and so nothing a filter's `search` can reach.
|
||||
// Routed to the scan that knows how to resolve it — and only for text that
|
||||
// could actually be one, so an ordinary query never pays for two scans.
|
||||
looksLikeAnEventId(term) -> LocalCache.search.findNotesStartingWith(term, account.hiddenUsers)
|
||||
else ->
|
||||
LocalCache.search.findNotesMatching(
|
||||
SearchFilterBuilder.build(parsed, limit = 200),
|
||||
account.hiddenUsers,
|
||||
)
|
||||
}
|
||||
val filtered = if (follows != null) raw.filter { it.author?.pubkeyHex in follows } else raw
|
||||
|
||||
@@ -407,6 +414,17 @@ class SearchBarViewModel(
|
||||
|
||||
override val isRefreshing = derivedStateOf { searchValue.isNotBlank() }
|
||||
|
||||
/**
|
||||
* Could this text name an event rather than describe one? A bech32 pointer, or a run of hex
|
||||
* long enough that it is nobody's search term.
|
||||
*/
|
||||
private fun looksLikeAnEventId(text: String): Boolean {
|
||||
val trimmed = text.trim()
|
||||
if (trimmed.isEmpty() || trimmed.contains(' ')) return false
|
||||
if (decodeEventIdAsHexOrNull(trimmed) != null) return true
|
||||
return trimmed.length >= 8 && trimmed.all { it in "0123456789abcdefABCDEF" }
|
||||
}
|
||||
|
||||
override fun invalidateData(ignoreIfDoing: Boolean) {
|
||||
// force new query
|
||||
invalidations.update { it + 1 }
|
||||
|
||||
Reference in New Issue
Block a user