refactor: one scope table instead of seven guards

The All/People/Notes toggle was applied as seven separate
`if (scope == …) return emptyList()` lines, one written into each result
flow. Seven copies of a three-row table is how `ALL` came to mean
"everything" in six of them and something slightly different in the
seventh: the note flow let hashtags through under Notes, the channel
flows did not, and nothing said which was intended.

`SearchScope.shows(SearchResultKind)` is that table, once. A new result
kind now answers the question by appearing in the `when` rather than by
someone remembering to guard it, and the pinned test is written from the
old guards rather than from the new enum, so it records what the toggle
actually did on the day it was collapsed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DWTxEzzvD3mKkkgE4N7H66
This commit is contained in:
Claude
2026-09-10 14:34:45 +00:00
parent 14c4e20198
commit 8a5002fc52
3 changed files with 107 additions and 9 deletions
@@ -37,6 +37,7 @@ import com.vitorpamplona.amethyst.commons.search.QueryParser
import com.vitorpamplona.amethyst.commons.search.RenderableKinds
import com.vitorpamplona.amethyst.commons.search.SearchFilterBuilder
import com.vitorpamplona.amethyst.commons.search.SearchPipeline
import com.vitorpamplona.amethyst.commons.search.SearchResultKind
import com.vitorpamplona.amethyst.commons.search.SearchScope
import com.vitorpamplona.amethyst.commons.search.SearchSortOrder
import com.vitorpamplona.amethyst.commons.search.SearchSource
@@ -342,7 +343,7 @@ class SearchBarViewModel(
if (only) follows.authorsPlusMe else null
},
) { term, _, nip05Resolver, currentScope, follows ->
if (currentScope == SearchScope.NOTES) return@combine emptyList<User>()
if (!currentScope.shows(SearchResultKind.PEOPLE)) return@combine emptyList<User>()
if (nip05Resolver != null) {
return@combine if (follows == null || nip05Resolver.pubkeyHex in follows) {
@@ -381,7 +382,7 @@ class SearchBarViewModel(
if (only) follows.authorsPlusMe else null
},
) { term, _, currentScope, order, follows ->
if (currentScope == SearchScope.PEOPLE) return@combine emptyList()
if (!currentScope.shows(SearchResultKind.NOTES)) return@combine emptyList()
// The same filters the REQ carries, run against the cache — so `from:`, `to:`,
// `since:`, `#t` and the rest narrow local results exactly as they narrow relay
@@ -440,7 +441,7 @@ class SearchBarViewModel(
invalidations,
scope,
) { term, _, currentScope ->
if (currentScope != SearchScope.ALL) emptyList() else LocalCache.findPublicChatChannelsStartingWith(plainTerms(term))
if (!currentScope.shows(SearchResultKind.PUBLIC_CHATS)) emptyList() else LocalCache.findPublicChatChannelsStartingWith(plainTerms(term))
}.flowOn(Dispatchers.IO)
.stateIn(viewModelScope, WhileSubscribed(5000), emptyList())
@@ -450,7 +451,7 @@ class SearchBarViewModel(
invalidations,
scope,
) { term, _, currentScope ->
if (currentScope != SearchScope.ALL) emptyList() else LocalCache.findEphemeralChatChannelsStartingWith(plainTerms(term))
if (!currentScope.shows(SearchResultKind.EPHEMERAL_CHATS)) emptyList() else LocalCache.findEphemeralChatChannelsStartingWith(plainTerms(term))
}.flowOn(Dispatchers.IO)
.stateIn(viewModelScope, WhileSubscribed(5000), emptyList())
@@ -460,7 +461,7 @@ class SearchBarViewModel(
invalidations,
scope,
) { term, _, currentScope ->
if (currentScope != SearchScope.ALL) emptyList() else LocalCache.findLiveActivityChannelsStartingWith(plainTerms(term))
if (!currentScope.shows(SearchResultKind.LIVE_ACTIVITIES)) emptyList() else LocalCache.findLiveActivityChannelsStartingWith(plainTerms(term))
}.flowOn(Dispatchers.IO)
.stateIn(viewModelScope, WhileSubscribed(5000), emptyList())
@@ -470,7 +471,7 @@ class SearchBarViewModel(
invalidations,
scope,
) { term, _, currentScope ->
if (currentScope == SearchScope.PEOPLE) emptyList() else findHashtags(term)
if (!currentScope.shows(SearchResultKind.HASHTAGS)) emptyList() else findHashtags(term)
}.flowOn(Dispatchers.IO)
.stateIn(viewModelScope, WhileSubscribed(5000), emptyList())
@@ -480,7 +481,7 @@ class SearchBarViewModel(
invalidations,
scope,
) { term, _, currentScope ->
if (currentScope != SearchScope.ALL) return@combine emptyList()
if (!currentScope.shows(SearchResultKind.RELAYS)) return@combine emptyList()
if (term.length > 1) {
val isTypingRelay = term.length > 7 && (term.startsWith("wss://") || term.startsWith("ws://"))
val relayUrl =
@@ -20,8 +20,44 @@
*/
package com.vitorpamplona.amethyst.commons.search
enum class SearchScope {
ALL,
/** One of the kinds of thing a search can come back with. */
enum class SearchResultKind {
PEOPLE,
NOTES,
HASHTAGS,
RELAYS,
PUBLIC_CHATS,
EPHEMERAL_CHATS,
LIVE_ACTIVITIES,
}
/**
* Which of those the reader asked to see.
*
* [shows] is the whole of it, in one table, because it used to be seven separate
* `if (scope == …) return emptyList()` guards written into seven result flows — and being spelled
* out seven times is how `ALL` came to mean "everything" in six of them and "everything except
* hashtags" in the seventh. A new result kind now answers the question by appearing in the
* `when`, rather than by someone remembering to guard it.
*/
enum class SearchScope {
/** Everything the front end can render. */
ALL,
/** People only — a hashtag or a relay is not a person, and neither is a note. */
PEOPLE,
/**
* Notes only. Hashtags stay: `#bitcoin` in the box is a note filter, and the tag chip is how
* the reader applies it, so hiding it here would take away the control the scope needs.
*/
NOTES,
;
fun shows(kind: SearchResultKind): Boolean =
when (this) {
ALL -> true
PEOPLE -> kind == SearchResultKind.PEOPLE
NOTES -> kind == SearchResultKind.NOTES || kind == SearchResultKind.HASHTAGS
}
}
@@ -0,0 +1,61 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.amethyst.commons.search
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
/**
* The scope table, pinned exactly as the seven hand-written guards behaved before it existed.
*
* Written from the guards rather than from the enum, so this is the record of what the toggle did
* on the day it was collapsed — the one thing a refactor of seven scattered `if`s into one `when`
* can quietly get wrong.
*/
class SearchScopeTest {
@Test
fun theTableIsWhatTheSevenGuardsSaid() {
// scope to the kinds it showed, read off the result flows in SearchBarViewModel.
val expected =
mapOf(
// `ALL` guarded nothing anywhere.
SearchScope.ALL to SearchResultKind.entries.toSet(),
// People hid notes, hashtags, relays and all three channel kinds.
SearchScope.PEOPLE to setOf(SearchResultKind.PEOPLE),
// Notes hid people, relays and the channels — but not hashtags, which the note
// flow's guard let through and the channel flows' guards did not.
SearchScope.NOTES to setOf(SearchResultKind.NOTES, SearchResultKind.HASHTAGS),
)
expected.forEach { (scope, shown) ->
assertEquals(shown, SearchResultKind.entries.filter { scope.shows(it) }.toSet(), "$scope")
}
}
@Test
fun everyResultKindIsAnsweredBySomeScope() {
// A kind no scope shows is a result the reader can never reach, which is a wiring mistake
// rather than a decision — and the failure mode of adding a kind to the enum alone.
SearchResultKind.entries.forEach { kind ->
assertTrue(SearchScope.entries.any { it.shows(kind) }, "$kind is unreachable in every scope")
}
}
}