From 8a0cf4548e7af04b7bae85f8cde8719b654495c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 00:33:49 +0000 Subject: [PATCH] fix(search): route only id-shaped text to the legacy scan, not every empty result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_017yKjw2WqwZpSzsqcYZMnkV --- .../plans/2026-09-07-generic-local-filter.md | 51 ++++++++++++++++++- amethyst/plans/README.md | 2 +- .../loggedIn/search/SearchBarViewModel.kt | 30 ++++++++--- 3 files changed, 75 insertions(+), 8 deletions(-) diff --git a/amethyst/plans/2026-09-07-generic-local-filter.md b/amethyst/plans/2026-09-07-generic-local-filter.md index 31952e98d5..a9fbb3624a 100644 --- a/amethyst/plans/2026-09-07-generic-local-filter.md +++ b/amethyst/plans/2026-09-07-generic-local-filter.md @@ -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. diff --git a/amethyst/plans/README.md b/amethyst/plans/README.md index b00cf90bf4..1aebbcf8fa 100644 --- a/amethyst/plans/README.md +++ b/amethyst/plans/README.md @@ -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. | diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt index 8bc3d0fa19..d600374901 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/search/SearchBarViewModel.kt @@ -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 }