From 546bd17b188ad833de7c82c03d24f66a40305a2f Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:12:03 +0300 Subject: [PATCH] fix(chat): snappy in-chat search over visible text only (#887) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(chat): snappy in-chat search over visible text only - findMatches now scans USER + ASSISTANT messages only — tool rows and system events were matched before, so navigation jumped to invisible matches and the scan paid for the chat's heaviest strings (tool payloads) - Reasoning text is explicitly out of scope (lives in reasoningText, never highlighted) — documented in the controller contract - Debounce (150ms) coalesces typing into one scan instead of one full scan per keystroke; blanking the query still clears instantly - Tests: controller scope (user/assistant only, tool/system/reasoning excluded, case-insensitive, navigation wrap) + delegate debounce (coalescing, cancelled intermediate queries, instant blank clear) * fix(chat): search navigation lands the match mid-view, not at an edge scrollToSearchMatch top-aligned the matched message with animateScrollToItem(index), pinning it to the viewport edge — tall messages left the highlighted word above/below the visible area. Now top-aligns, then nudges the item down to ~1/3 viewport height (verified top-align -> measure -> adjust pattern, same as scrollToBottom). * fix(chat): search scroll targets the real lazy item and the actual word ROOT CAUSE: search-match indices are MESSAGE-list indices, but the LazyColumn has extra items (reasoning hoists, tool rows, loading-older), so scrollToSearchMatch(index) landed on the WRONG item whenever reasoning or tools preceded the match — the 'does nothing / word under or above the view' bug from the start. - messageIdToLazyIndex(): pure messageId -> LazyColumn item index mapping mirroring FullBleedChatList's emission order (verified in tests) - search-scroll effect moved INTO FullBleedChatList (the only composable that knows the mapping), serialized through the same ChatScrollController - word-level targeting: each SearchMatch now carries its character offset in the message; the scroll places offset/contentLength x item height at ~1/3 viewport, so the highlighted word itself lands in view on tall or multi-occurrence messages - counter/navigation now count OCCURRENCES (word hits), one entry per hit with a parallel searchMatchOffsets list; per-message dedupe removed - tests: lazy-index mapping (reasoning hoist, tools, leading items), SearchMatch offsets, occurrence flattening, regex-literal queries * fix(ci): pass searchMatchOffsets and scrollController to FullBleedChatList in instrumented test The render helper in FullBleedChatListTest still used the pre-offset signature, so compileDebugAndroidTestKotlin failed on the emulator job. Verified locally with compileDebugAndroidTestKotlin + ktlintCheck. * perf(chat): allocation-free indexOf search matcher with capped results - findMatches scans with String.indexOf(ignoreCase=true) on the ORIGINAL content instead of Regex(Pattern.quote()).findAll: queries are always literal, so regex was pure overhead — a compiled Pattern plus one MatchResult allocation per hit. Offsets stay exact (no lowercase transform, which can change string length on some scripts). - returns SearchResult(matches, totalMatches, capped): results are capped at 500 stored entries (MAX_SEARCH_MATCHES) to bound allocations and navigation on degenerate queries, while the counter still shows the exact total (searchMatchTotal/searchMatchCapped in ChatUiState). - 14 controller tests green, delegate behavior unchanged. * perf(chat): O(1) current-match lookup and highlight only matched bubbles Two recomposition hot spots killed in the list renderer: - isCurrentMatchFor() did messages.indexOfFirst per rendered bubble per search update — O(n) per item, O(n^2) per update on the main thread. Now currentMatchMessageId() resolves the current match's message id ONCE per recomposition and items compare ids (O(1)). - every prose bubble re-ran its highlight scan on every search-state change even with zero hits. matchedMessageIds() computes the matched id set once; bubbles outside it get an empty searchQuery and skip the scan entirely. Both helpers are pure, tested in FullBleedTurnsTest (18 tests), and the instrumented test target still compiles. * perf(chat): dedicated ChatSearchState holder — search no longer recomposes the list Search fields lived in ChatUiState, so every debounced scan result changed the whole state object: ChatScreen recomposed, FullBleedChatList's params changed, and the entire list body re-ran (turns loop + item registrations) on the main thread per search update. Search state now lives in a @Stable snapshot-backed ChatSearchState (LazyListState pattern) owned by ChatSearchDelegate: - the list takes ONE stable param; the instance never changes, so the body is skipped on search updates — only the scopes that read a field recompose (search bar, matched bubbles, the scroll effect) - matchedIds and currentMatchId are computed once per scan/navigate in the delegate and stored as state fields — items do O(1) id lookups inside their own lambdas, no body-level derivation - ChatUiState loses the 7 search fields; ChatViewModel exposes viewModel.searchState; delegate tests now assert on the holder 19 search-related unit tests green, androidTest compiles. * test(chat): long-session search sanity (2000 msgs) — exact counts, valid offsets Synthetic 2000-message / ~1KB session verifying the matcher at scale: - frequent word: exact 32_000 total, capped storage, every stored offset points at a real occurrence - unique word resolves to the right message; tool payloads stay excluded - local measurement: full scan ~7ms (regex version allocates per hit) --------- Co-authored-by: M57 --- .../chat/fullbleed/FullBleedChatListTest.kt | 17 +- .../m57/hermescontrol/ui/chat/ChatScreen.kt | 25 ++- .../ui/chat/ChatSearchController.kt | 66 ++++++- .../ui/chat/ChatSearchDelegate.kt | 119 +++++++----- .../hermescontrol/ui/chat/ChatSearchState.kt | 57 ++++++ .../hermescontrol/ui/chat/ChatViewModel.kt | 15 +- .../chat/components/ChatLifecycleEffects.kt | 16 -- .../chat/components/ChatScrollController.kt | 33 +++- .../ui/chat/components/ChatSearchBar.kt | 5 +- .../ui/chat/fullbleed/FullBleedChatList.kt | 86 +++++---- .../ui/chat/fullbleed/FullBleedTurns.kt | 86 +++++++++ .../ui/chat/ChatSearchControllerTest.kt | 179 ++++++++++++++++++ .../ui/chat/ChatSearchDelegateTest.kt | 171 +++++++++++++++++ .../ui/chat/ChatSearchPerfSanityTest.kt | 91 +++++++++ .../ui/chat/ChatViewModelTest.kt | 8 +- .../ui/chat/FullBleedTurnsTest.kt | 63 ++++++ 16 files changed, 895 insertions(+), 142 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchState.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchControllerTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegateTest.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchPerfSanityTest.kt diff --git a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt index 30514278..d94af3c5 100644 --- a/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt +++ b/app/src/androidTest/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatListTest.kt @@ -9,10 +9,14 @@ import androidx.compose.ui.test.onNodeWithTag import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatSearchState import com.m57.hermescontrol.ui.chat.ChatViewModel import com.m57.hermescontrol.ui.chat.MessageRole import com.m57.hermescontrol.ui.chat.ToolStatus +import com.m57.hermescontrol.ui.chat.components.ChatScrollController import io.mockk.mockk +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -46,19 +50,22 @@ class FullBleedChatListTest { clarify: Boolean = false, ) { composeTestRule.setContent { + val listState = LazyListState() FullBleedChatList( messages = messages, streamingMessage = streamingMessage, - isSearchActive = false, - searchQuery = "", - currentSearchMatchIndex = -1, - searchMatchIndices = emptyList(), + searchState = ChatSearchState(), typingEffectEnabled = false, typingEffectDelayMs = 30, isLoading = false, isLoadingOlder = false, isDark = false, - listState = LazyListState(), + listState = listState, + scrollController = + ChatScrollController( + listState = listState, + scope = CoroutineScope(Dispatchers.Main.immediate), + ), lastAnimatedMessageId = null, onLastAnimatedMessageIdChange = {}, viewModel = mockk(relaxed = true), diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt index d630fc11..2445f3ad 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatScreen.kt @@ -146,6 +146,9 @@ fun ChatScreen( val state by viewModel.uiState.collectAsStateWithLifecycle() val streamingState by viewModel.streamingState.collectAsStateWithLifecycle() val credentialWarning by HermesWsClient.credentialWarning.collectAsStateWithLifecycle() + // Snapshot-backed search state — read directly so only the scopes that + // read its fields recompose on search changes (bar, matched bubbles). + val searchState = viewModel.searchState val lifecycleOwner = LocalLifecycleOwner.current val listState = rememberLazyListState() val scrollScope = rememberCoroutineScope() @@ -402,9 +405,6 @@ fun ChatScreen( errorMessage = state.errorMessage, backgroundCompleteMessage = state.backgroundCompleteMessage, openError = state.openError, - isSearchActive = state.isSearchActive, - currentSearchMatchIndex = state.currentSearchMatchIndex, - searchMatchIndices = state.searchMatchIndices, clarifyRequest = state.clarifyRequest, sudoPrompt = state.sudoPrompt, secretPrompt = state.secretPrompt, @@ -477,9 +477,9 @@ fun ChatScreen( IconButton(onClick = { viewModel.toggleSearch() }) { Icon( imageVector = - if (state.isSearchActive) Icons.Filled.Close else Icons.Filled.Search, + if (searchState.isActive) Icons.Filled.Close else Icons.Filled.Search, contentDescription = - if (state.isSearchActive) { + if (searchState.isActive) { stringResource( R.string.chat_action_close_search, ) @@ -519,7 +519,7 @@ fun ChatScreen( } AnimatedVisibility( - visible = state.isSearchActive, + visible = searchState.isActive, enter = expandVertically() + fadeIn(), exit = shrinkVertically() + fadeOut(), ) { @@ -535,10 +535,11 @@ fun ChatScreen( ) { Box(modifier = Modifier.padding(horizontal = 12.dp, vertical = 8.dp)) { SearchBarRow( - searchQuery = state.searchQuery, + searchQuery = searchState.query, onQueryChange = { viewModel.setSearchQuery(it) }, - searchMatchCount = state.searchMatchIndices.size, - currentMatchIndex = state.currentSearchMatchIndex, + searchMatchCount = searchState.matchTotal, + searchMatchCapped = searchState.matchCapped, + currentMatchIndex = searchState.currentIndex, onNavigateUp = { viewModel.navigateSearchMatch(-1) }, onNavigateDown = { viewModel.navigateSearchMatch(1) }, onClose = { viewModel.clearSearch() }, @@ -583,10 +584,7 @@ fun ChatScreen( FullBleedChatList( messages = state.messages, streamingMessage = streamingState.streamingMessage, - isSearchActive = state.isSearchActive, - searchQuery = state.searchQuery, - currentSearchMatchIndex = state.currentSearchMatchIndex, - searchMatchIndices = state.searchMatchIndices, + searchState = searchState, typingEffectEnabled = state.typingEffectEnabled, typingEffectDelayMs = state.typingEffectDelayMs, maxToolCallsPerTurn = state.maxToolCallsPerTurn, @@ -594,6 +592,7 @@ fun ChatScreen( isLoadingOlder = state.isLoadingOlder, isDark = isDark, listState = listState, + scrollController = scrollController, lastAnimatedMessageId = lastAnimatedMessageId, onLastAnimatedMessageIdChange = { lastAnimatedMessageId = it }, viewModel = viewModel, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt index 5255dd26..0548af40 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchController.kt @@ -1,5 +1,22 @@ package com.m57.hermescontrol.ui.chat +/** + * One search hit: which message it lives in and where in that message's + * visible content the word starts (character offset). The offset lets the + * chat list scroll the actual word into view, not just its message. + */ +data class SearchMatch( + val messageIndex: Int, + val contentOffset: Int, +) + +/** Result of a search scan, including the exact total before any cap. */ +data class SearchResult( + val matches: List, + val totalMatches: Int, + val capped: Boolean, +) + /** * Pure search logic for in-chat text search. * @@ -7,16 +24,50 @@ package com.m57.hermescontrol.ui.chat */ class ChatSearchController { /** - * Find all message indices where [query] appears in the content. + * Find all WORD OCCURRENCES of [query] in the conversation's VISIBLE + * text — user bubbles and agent prose only — flattened into one entry per + * occurrence, carrying each hit's character offset in its message. + * + * Tool rows, system events, and reasoning text are excluded: + * - they are not visible prose the user is looking for, so matches there + * were invisible (nothing highlighted) yet still navigated/scrolled to + * - tool payloads dominate the chat's byte weight, so scanning them made + * search slow on long sessions + * Reasoning lives in [ChatMessage.reasoningText] (not `content`), so it + * was never matched — this keeps it that way explicitly. + * + * Matching is a literal, allocation-light [String.indexOf] scan + * (`ignoreCase = true`) instead of a regex — queries are always quoted + * literally, so regex was pure overhead: one MatchResult allocation per + * hit plus a compiled Pattern per query. Scanning the ORIGINAL content + * keeps offsets exact (no lowercase transform, which can change string + * length on some scripts). + * + * Result is capped at [MAX_SEARCH_MATCHES] entries to bound allocations + * and navigation cost on degenerate queries (single letter, "the"); + * [SearchResult.totalMatches] still reports the exact total. */ fun findMatches( messages: List, query: String, - ): List { - if (query.isBlank()) return emptyList() - return messages.indices.filter { idx -> - messages[idx].content.contains(query, ignoreCase = true) + ): SearchResult { + if (query.isBlank()) return SearchResult(emptyList(), 0, capped = false) + val result = mutableListOf() + var total = 0 + for ((idx, message) in messages.withIndex()) { + if (message.role != MessageRole.USER && message.role != MessageRole.ASSISTANT) continue + var from = 0 + while (true) { + val hit = message.content.indexOf(query, from, ignoreCase = true) + if (hit < 0) break + total++ + if (result.size < MAX_SEARCH_MATCHES) { + result.add(SearchMatch(idx, hit)) + } + from = hit + query.length + } } + return SearchResult(result, total, capped = total > MAX_SEARCH_MATCHES) } /** @@ -34,4 +85,9 @@ class ChatSearchController { else -> currentIndex } } + + companion object { + /** Hard cap on stored match entries; totals beyond it show as `N+`. */ + const val MAX_SEARCH_MATCHES = 500 + } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt index decf0463..913de983 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt @@ -1,103 +1,120 @@ package com.m57.hermescontrol.ui.chat +import com.m57.hermescontrol.ui.chat.fullbleed.currentMatchMessageId +import com.m57.hermescontrol.ui.chat.fullbleed.matchedMessageIds import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch /** * Holds chat in-message search state and logic, extracted from [ChatViewModel] * to keep the god-object focused on messaging/session concerns. * - * Behavior is identical to the original inline implementation: it mutates the - * shared [uiState] flow for the four search-related fields and reads `messages` - * from it when computing matches. + * Search state lives in a dedicated snapshot-backed [ChatSearchState] holder + * (NOT in [ChatUiState]): search updates then recompose only the scopes that + * read its fields (search bar, matched bubbles, the scroll effect) instead of + * the entire chat screen and list. [uiState] is still read for `messages` + * when computing matches. * * @param dispatcher The [CoroutineDispatcher] used for the (CPU-bound) search * work. Defaults to [Dispatchers.Default] — the original behavior — but can * be injected to reuse a caller's context or customize per environment. + * @param debounceMs How long to wait after the last keystroke before running + * the scan. Coalesces fast typing into one search instead of one full scan + * per character. */ class ChatSearchDelegate( private val scope: CoroutineScope, private val uiState: MutableStateFlow, + val searchState: ChatSearchState = ChatSearchState(), private val searchController: ChatSearchController = ChatSearchController(), private val dispatcher: CoroutineDispatcher = Dispatchers.Default, + private val debounceMs: Long = SEARCH_DEBOUNCE_MS, ) { private var searchJob: Job? = null fun toggleSearch() { - val current = uiState.value - if (current.isSearchActive) { + if (searchState.isActive) { clearSearch() } else { - uiState.update { it.copy(isSearchActive = true, searchQuery = "") } + searchState.isActive = true + searchState.query = "" } } fun setSearchQuery(query: String) { searchJob?.cancel() + searchState.query = query if (query.isBlank()) { - uiState.update { - it.copy( - searchQuery = query, - searchMatchIndices = emptyList(), - currentSearchMatchIndex = -1, - ) - } + resetMatches() return } - // Keep local state in sync immediately so UI feels responsive. - uiState.update { - it.copy(searchQuery = query) - } - - searchJob = scope.launch(dispatcher) { runSearch(query) } + // Keep local state in sync immediately so UI feels responsive, then + // debounce: cancel any pending scan and only run one after typing + // pauses for debounceMs (fast typing = one scan, not one per char). + searchJob = + scope.launch(dispatcher) { + delay(debounceMs) + runSearch(query) + } } private fun runSearch(query: String) { val messages = uiState.value.messages - val indices = searchController.findMatches(messages, query) + val result = searchController.findMatches(messages, query) - uiState.update { - // Only update indices if the search query hasn't changed in the meantime - if (it.searchQuery == query) { - it.copy( - searchMatchIndices = indices, - currentSearchMatchIndex = if (indices.isNotEmpty()) 0 else -1, - ) - } else { - it - } - } + // Only update matches if the search query hasn't changed in the meantime. + if (searchState.query != query) return + + searchState.matchIndices = result.matches.map { m -> m.messageIndex } + searchState.matchOffsets = result.matches.map { m -> m.contentOffset } + searchState.matchTotal = result.totalMatches + searchState.matchCapped = result.capped + searchState.currentIndex = if (result.matches.isNotEmpty()) 0 else -1 + searchState.matchedIds = matchedMessageIds(messages, searchState.matchIndices) + searchState.currentMatchId = + currentMatchMessageId(messages, searchState.matchIndices, searchState.currentIndex) } fun navigateSearchMatch(direction: Int) { - uiState.update { state -> - val indices = state.searchMatchIndices - if (indices.isEmpty()) return@update state - val newIdx = - searchController.navigate( - currentIndex = state.currentSearchMatchIndex, - matchCount = indices.size, - direction = direction, - ) - state.copy(currentSearchMatchIndex = newIdx) - } + val indices = searchState.matchIndices + if (indices.isEmpty()) return + val newIdx = + searchController.navigate( + currentIndex = searchState.currentIndex, + matchCount = indices.size, + direction = direction, + ) + searchState.currentIndex = newIdx + searchState.currentMatchId = + currentMatchMessageId(uiState.value.messages, indices, newIdx) } fun clearSearch() { - uiState.update { - it.copy( - isSearchActive = false, - searchQuery = "", - searchMatchIndices = emptyList(), - currentSearchMatchIndex = -1, - ) - } + searchJob?.cancel() + searchState.isActive = false + searchState.query = "" + resetMatches() + } + + private fun resetMatches() { + searchState.matchIndices = emptyList() + searchState.matchOffsets = emptyList() + searchState.matchTotal = 0 + searchState.matchCapped = false + searchState.currentIndex = -1 + searchState.matchedIds = emptySet() + searchState.currentMatchId = null + } + + private companion object { + /** Typing pause (ms) before a search scan runs. */ + const val SEARCH_DEBOUNCE_MS = 150L } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchState.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchState.kt new file mode 100644 index 00000000..fd6d4114 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchState.kt @@ -0,0 +1,57 @@ +package com.m57.hermescontrol.ui.chat + +import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue + +/** + * Snapshot-backed holder for in-chat search state, owned by + * [ChatSearchDelegate] and read directly by composables. + * + * Being a STABLE instance with mutable [androidx.compose.runtime.State] + * fields — the LazyListState pattern — means search updates never change + * the instance itself, so composables that take it as a parameter (the + * full-bleed chat list) are SKIPPED on search changes; only the scopes + * that actually read a field (visible item lambdas, the scroll effect) + * recompose. That was the last big cost: search state used to live inside + * [ChatUiState], so every debounced scan re-executed the entire list body + * (turns loop + item registrations) on the main thread. + */ +@Stable +class ChatSearchState { + var isActive by mutableStateOf(false) + internal set + + var query by mutableStateOf("") + internal set + + /** Stored matches (capped at [ChatSearchController.MAX_SEARCH_MATCHES]). */ + var matchIndices by mutableStateOf(emptyList()) + internal set + + /** Parallel to [matchIndices]: character offset of each occurrence. */ + var matchOffsets by mutableStateOf(emptyList()) + internal set + + var currentIndex by mutableStateOf(-1) + internal set + + /** Exact total before the cap; [matchCapped] renders the counter as `N+`. */ + var matchTotal by mutableStateOf(0) + internal set + + var matchCapped by mutableStateOf(false) + internal set + + /** + * Ids of messages containing at least one match. Bubbles outside this + * set skip their highlight scan entirely. + */ + var matchedIds by mutableStateOf(emptySet()) + internal set + + /** Id of the message holding the CURRENT match (O(1) highlight lookup). */ + var currentMatchId by mutableStateOf(null) + internal set +} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index 0aa8d291..0c88a952 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -265,11 +265,8 @@ data class ChatUiState( val showSessionPicker: Boolean = false, // /update confirm dialog (issue #862) — the command is handled client-side val updateConfirmOpen: Boolean = false, - // Search state - val isSearchActive: Boolean = false, - val searchQuery: String = "", - val searchMatchIndices: List = emptyList(), - val currentSearchMatchIndex: Int = -1, + // Search state lives in ChatSearchState (searchDelegate.searchState) — a + // snapshot-backed holder, so search updates don't recompose the whole UI. // Cached settings val typingEffectEnabled: Boolean = true, val typingEffectDelayMs: Int = 30, @@ -481,6 +478,10 @@ class ChatViewModel( uiState = _uiState, dispatcher = searchDispatcher, ) + + /** Snapshot-backed in-chat search state (see [ChatSearchDelegate]). */ + val searchState: ChatSearchState + get() = searchDelegate.searchState private val attachmentsDelegate = ChatAttachmentsDelegate(uiState = _uiState) /** @@ -2112,10 +2113,6 @@ class ChatViewModel( sudoPrompt = null, secretPrompt = null, showSessionPicker = false, - isSearchActive = false, - searchQuery = "", - searchMatchIndices = emptyList(), - currentSearchMatchIndex = -1, showModelPicker = false, modelPickerLoading = false, currentSessionModel = null, diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatLifecycleEffects.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatLifecycleEffects.kt index af9425bc..f2909116 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatLifecycleEffects.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatLifecycleEffects.kt @@ -32,9 +32,6 @@ fun ChatLifecycleEffects( errorMessage: String?, backgroundCompleteMessage: String?, openError: String?, - isSearchActive: Boolean, - currentSearchMatchIndex: Int, - searchMatchIndices: List, clarifyRequest: ClarifyUi?, sudoPrompt: SudoPromptUi?, secretPrompt: SecretPromptUi?, @@ -134,17 +131,4 @@ fun ChatLifecycleEffects( onDismiss = viewModel::dismissSecret, ) } - - // Scroll to current search match (serialized through the controller so it - // doesn't compete with auto-follow / FAB / send scrolls). - LaunchedEffect(isSearchActive, currentSearchMatchIndex, searchMatchIndices) { - if (isSearchActive && - currentSearchMatchIndex >= 0 && - currentSearchMatchIndex < searchMatchIndices.size && - messages.isNotEmpty() - ) { - val targetIndex = searchMatchIndices[currentSearchMatchIndex] - scrollController.scrollToSearchMatch(targetIndex.coerceIn(0, messages.lastIndex)) - } - } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt index c01f8cd4..b37e39cb 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatScrollController.kt @@ -151,9 +151,36 @@ class ChatScrollController( scope.launch { listState.scrollToItem(index, offset) } } - /** Navigate to a search match index (serialized through [scope]). */ - fun scrollToSearchMatch(index: Int) { - scope.launch { listState.animateScrollToItem(index) } + /** + * Navigate to a search match (serialized through [scope]). + * + * [index] is the LAZY-COLUMN item index (see [messageIdToLazyIndex] — + * message indices ≠ lazy indices once reasoning/tool items exist). + * Top-aligns the item, then repositions it so the matched WORD lands + * comfortably in view (~1/3 down the viewport): the word's position is + * estimated as textOffset / contentLength × measured item height. Mirrors + * the verified scrollToBottom pattern (top-align → measure → adjust). + * + * @param contentOffset character offset of the word in the message text. + * @param contentLength total length of the message text. + */ + fun scrollToSearchMatch( + index: Int, + contentOffset: Int, + contentLength: Int, + ) { + scope.launch { + listState.animateScrollToItem(index) + val info = listState.layoutInfo + val item = info.visibleItemsInfo.firstOrNull { it.index == index } ?: return@launch + val viewportHeight = info.viewportEndOffset - info.viewportStartOffset + val fraction = if (contentLength > 0) contentOffset.toFloat() / contentLength else 0f + val targetTop = (viewportHeight / 3) - (item.size * fraction).toInt() + val delta = item.offset - targetTop + if (delta != 0) { + listState.animateScrollBy(delta.toFloat()) + } + } } } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatSearchBar.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatSearchBar.kt index 801b6147..c5209bd5 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatSearchBar.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/components/ChatSearchBar.kt @@ -101,6 +101,7 @@ fun SearchBarRow( searchQuery: String, onQueryChange: (String) -> Unit, searchMatchCount: Int, + searchMatchCapped: Boolean = false, currentMatchIndex: Int, onNavigateUp: () -> Unit, onNavigateDown: () -> Unit, @@ -124,7 +125,9 @@ fun SearchBarRow( if (searchQuery.isNotEmpty()) { val countText = if (searchMatchCount > 0) { - "${currentMatchIndex + 1}/$searchMatchCount" + // `500+` when the exact total exceeded the stored cap. + val total = if (searchMatchCapped) "$searchMatchCount+" else "$searchMatchCount" + "${currentMatchIndex + 1}/$total" } else { "0/0" } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt index a2d53402..d126cc49 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedChatList.kt @@ -10,6 +10,7 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.material3.CircularProgressIndicator import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -18,10 +19,12 @@ import androidx.compose.ui.unit.dp import com.m57.hermescontrol.R import com.m57.hermescontrol.ui.chat.ChatBubble import com.m57.hermescontrol.ui.chat.ChatMessage +import com.m57.hermescontrol.ui.chat.ChatSearchState import com.m57.hermescontrol.ui.chat.ChatViewModel import com.m57.hermescontrol.ui.chat.ClarifyUi import com.m57.hermescontrol.ui.chat.ImageViewerModel import com.m57.hermescontrol.ui.chat.ToolCallDivider +import com.m57.hermescontrol.ui.chat.components.ChatScrollController import com.m57.hermescontrol.ui.chat.components.ClarifyBubble import com.m57.hermescontrol.ui.chat.components.ReasoningCard import com.m57.hermescontrol.ui.chat.toolCallMilestones @@ -46,10 +49,7 @@ import com.m57.hermescontrol.ui.common.EmptyState fun FullBleedChatList( messages: List, streamingMessage: ChatMessage?, - isSearchActive: Boolean, - searchQuery: String, - currentSearchMatchIndex: Int, - searchMatchIndices: List, + searchState: ChatSearchState, typingEffectEnabled: Boolean, typingEffectDelayMs: Int, maxToolCallsPerTurn: Int? = null, @@ -57,6 +57,7 @@ fun FullBleedChatList( isLoadingOlder: Boolean, isDark: Boolean, listState: androidx.compose.foundation.lazy.LazyListState, + scrollController: ChatScrollController, lastAnimatedMessageId: String?, onLastAnimatedMessageIdChange: (String?) -> Unit, viewModel: ChatViewModel, @@ -83,6 +84,37 @@ fun FullBleedChatList( remember(messages, streamingMessage) { groupIntoTurnsWithStreaming(messages, streamingMessage) } + // messageId → LazyColumn item index. The lazy list has EXTRA items + // vs the message list (reasoning hoists, tool rows), so search-match + // scrolling must resolve the lazy index, not use the message index. + val lazyIndexById = + remember(turns, isLoadingOlder) { + messageIdToLazyIndex(turns, leadingItems = if (isLoadingOlder) 1 else 0) + } + + // Scroll the current search match into view, word-focused. Lives here + // (not in ChatLifecycleEffects) because only this composable knows + // the message-id → lazy-item-index mapping. Reads search fields in + // the effect (not the body), so only this effect restarts on change. + LaunchedEffect( + searchState.isActive, + searchState.currentIndex, + searchState.matchIndices, + searchState.matchOffsets, + ) { + if (searchState.isActive && + searchState.currentIndex >= 0 && + searchState.currentIndex < searchState.matchIndices.size + ) { + val messageIndex = searchState.matchIndices[searchState.currentIndex] + if (messageIndex < 0 || messageIndex >= messages.size) return@LaunchedEffect + val lazyIndex = lazyIndexById[messages[messageIndex].id] ?: return@LaunchedEffect + val contentOffset = searchState.matchOffsets.getOrElse(searchState.currentIndex) { 0 } + val contentLength = messages[messageIndex].content.length + scrollController.scrollToSearchMatch(lazyIndex, contentOffset, contentLength) + } + } + LazyColumn( state = listState, modifier = Modifier.fillMaxSize(), @@ -111,15 +143,10 @@ fun FullBleedChatList( Column(modifier = Modifier.padding(bottom = 12.dp)) { renderChatBubble( message = userMessage, - searchQuery = searchQuery, + searchQuery = if (searchState.isActive) searchState.query else "", isCurrentMatch = - isCurrentMatchFor( - messages, - userMessage.id, - isSearchActive, - currentSearchMatchIndex, - searchMatchIndices, - ), + searchState.currentMatchId != null && + searchState.currentMatchId == userMessage.id, onOpenAttachment = viewModel::openAttachment, onSaveAttachment = onSaveAttachment, savingAttachmentPath = savingAttachmentPath, @@ -181,15 +208,19 @@ fun FullBleedChatList( message = proseMessage, showTurnHeader = showTurnHeader, isDarkTheme = isDark, - searchQuery = if (isSearchActive) searchQuery else "", + // Highlight only bubbles that actually contain a match — + // the rest skip the highlight scan entirely. + searchQuery = + if (searchState.isActive && + proseMessage.id in searchState.matchedIds + ) { + searchState.query + } else { + "" + }, isCurrentMatch = - isCurrentMatchFor( - messages, - proseMessage.id, - isSearchActive, - currentSearchMatchIndex, - searchMatchIndices, - ), + searchState.currentMatchId != null && + searchState.currentMatchId == proseMessage.id, showReasoning = !hoistedReasoning, onOpenAttachment = viewModel::openAttachment, onSaveAttachment = onSaveAttachment, @@ -275,18 +306,3 @@ private fun renderChatBubble( onImageClick = onImageClick, ) } - -private fun isCurrentMatchFor( - messages: List, - messageId: String, - isSearchActive: Boolean, - currentSearchMatchIndex: Int, - searchMatchIndices: List, -): Boolean { - if (!isSearchActive || currentSearchMatchIndex < 0 || currentSearchMatchIndex >= searchMatchIndices.size) { - return false - } - val index = messages.indexOfFirst { it.id == messageId } - if (index < 0) return false - return searchMatchIndices[currentSearchMatchIndex] == index -} diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt index 9eb71dad..a60e3346 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/fullbleed/FullBleedTurns.kt @@ -88,3 +88,89 @@ fun groupIntoTurnsWithStreaming( } return turns } + +/** + * Map every visible chat message id to its LazyColumn item index in the + * full-bleed renderer. + * + * The lazy list does NOT have one item per message: each agent turn with a + * reasoning block emits an extra `reasoning-` item BEFORE its prose, and + * tool rows / system events are items too. Scrolling a search match by raw + * message index therefore lands on the WRONG item whenever reasoning or tool + * rows precede the target — the classic "match is above/below the view" bug. + * + * Mirrors the item emission order in [FullBleedChatList] exactly: + * user turn → 1 item; agent turn → optional reasoning item, then one item per + * entry. [leadingItems] accounts for fixed items emitted before the turns + * (e.g. the `loading-older` spinner when paging). + * + * @return messageId → LazyColumn item index of the message's content item + * (prose items for agent messages; user items for user messages). + */ +fun messageIdToLazyIndex( + turns: List, + leadingItems: Int = 0, +): Map { + val map = mutableMapOf() + var itemIndex = leadingItems + turns.forEach { turn -> + when (turn) { + is ChatTurn.User -> { + map[turn.message.id] = itemIndex + itemIndex++ + } + + is ChatTurn.Agent -> { + val hasReasoning = + turn.entries + .filterIsInstance() + .any { it.message.reasoningText.isNotBlank() } + if (hasReasoning) { + // The reasoning- hoist item occupies one slot. + itemIndex++ + } + turn.entries.forEach { entry -> + when (entry) { + is AgentEntry.Prose -> { + map[entry.message.id] = itemIndex + itemIndex++ + } + + is AgentEntry.ToolRow -> itemIndex++ + is AgentEntry.SystemEvent -> itemIndex++ + } + } + } + } + } + return map +} + +/** + * Resolve the message id of the CURRENT search match once, so per-item + * highlight lookups are O(1) id comparisons instead of O(n) linear scans + * (`messages.indexOfFirst` per rendered bubble was O(n²) per search update). + */ +fun currentMatchMessageId( + messages: List, + searchMatchIndices: List, + currentSearchMatchIndex: Int, +): String? { + if (currentSearchMatchIndex < 0 || currentSearchMatchIndex >= searchMatchIndices.size) return null + val messageIndex = searchMatchIndices[currentSearchMatchIndex] + if (messageIndex < 0 || messageIndex >= messages.size) return null + return messages[messageIndex].id +} + +/** + * Message ids that contain at least one search match. Bubbles outside this + * set skip their highlight scan entirely (they used to re-run it on every + * search-state change even with zero hits). + */ +fun matchedMessageIds( + messages: List, + searchMatchIndices: List, +): Set = + searchMatchIndices + .mapNotNull { messages.getOrNull(it)?.id } + .toSet() diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchControllerTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchControllerTest.kt new file mode 100644 index 00000000..b35bea74 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchControllerTest.kt @@ -0,0 +1,179 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ChatSearchControllerTest { + private val controller = ChatSearchController() + + private fun message( + role: MessageRole, + content: String, + reasoningText: String = "", + ): ChatMessage = ChatMessage(role = role, content = content, reasoningText = reasoningText) + + private fun hits(result: SearchResult) = result.matches + + @Test + fun `matches user and assistant visible text with content offsets`() { + val messages = + listOf( + message(MessageRole.USER, "deploy the API"), + message(MessageRole.ASSISTANT, "Let's deploy it now"), + message(MessageRole.USER, "unrelated"), + ) + + assertEquals( + listOf(SearchMatch(0, 0), SearchMatch(1, 6)), + hits(controller.findMatches(messages, "deploy")), + ) + assertEquals(2, controller.findMatches(messages, "deploy").totalMatches) + assertFalse(controller.findMatches(messages, "deploy").capped) + } + + @Test + fun `excludes tool rows even when the payload contains the query`() { + val messages = + listOf( + message(MessageRole.USER, "check the logs"), + message(MessageRole.TOOL, "{\"result\": \"check the logs: all green\"}"), + message(MessageRole.ASSISTANT, "all good"), + ) + + assertEquals(listOf(SearchMatch(0, 0)), hits(controller.findMatches(messages, "check the logs"))) + } + + @Test + fun `excludes system events`() { + val messages = + listOf( + message(MessageRole.USER, "approve the terminal command"), + message(MessageRole.SYSTEM, "Approval requested: approve the terminal command"), + ) + + assertEquals(listOf(SearchMatch(0, 0)), hits(controller.findMatches(messages, "approve"))) + } + + @Test + fun `excludes reasoning text that never made it to visible content`() { + val messages = + listOf( + message( + role = MessageRole.ASSISTANT, + content = "Here is the answer", + reasoningText = "thinking about the deployment steps", + ), + ) + + // Only the visible content is searchable — reasoning is invisible. + assertEquals(emptyList(), hits(controller.findMatches(messages, "deployment"))) + assertEquals(listOf(SearchMatch(0, 12)), hits(controller.findMatches(messages, "answer"))) + } + + @Test + fun `matching is case insensitive`() { + val messages = listOf(message(MessageRole.USER, "Deploy the API now")) + + assertEquals(listOf(SearchMatch(0, 0)), hits(controller.findMatches(messages, "dEpLoY"))) + } + + @Test + fun `repeats the message index once per occurrence with each offset`() { + val messages = listOf(message(MessageRole.USER, "deploy deploy and deploy")) + + // 3 hits in one message = 3 entries with their own character offsets. + assertEquals( + listOf(SearchMatch(0, 0), SearchMatch(0, 7), SearchMatch(0, 18)), + hits(controller.findMatches(messages, "deploy")), + ) + } + + @Test + fun `counts occurrences across multiple messages`() { + val messages = + listOf( + message(MessageRole.USER, "deploy the api"), + message(MessageRole.ASSISTANT, "deploy deploy"), + ) + + assertEquals( + listOf(SearchMatch(0, 0), SearchMatch(1, 0), SearchMatch(1, 7)), + hits(controller.findMatches(messages, "deploy")), + ) + } + + @Test + fun `regex special characters in the query are treated literally`() { + val messages = listOf(message(MessageRole.USER, "a+b c a+b")) + + assertEquals( + listOf(SearchMatch(0, 0), SearchMatch(0, 6)), + hits(controller.findMatches(messages, "a+b")), + ) + assertEquals(emptyList(), hits(controller.findMatches(messages, "a."))) + } + + @Test + fun `blank query returns no matches`() { + val messages = listOf(message(MessageRole.USER, "deploy")) + + assertEquals(emptyList(), hits(controller.findMatches(messages, " "))) + } + + @Test + fun `matches every occurrence in a long message with correct offsets`() { + val content = "deploy ".repeat(200) + "deploy" + val m = message(MessageRole.USER, content) + + val result = controller.findMatches(listOf(m), "deploy") + assertEquals(201, result.matches.size) + assertEquals(201, result.totalMatches) + assertEquals(0, result.matches.first().contentOffset) + assertEquals(1400, result.matches.last().contentOffset) + assertFalse(result.capped) + } + + @Test + fun `case-insensitive match keeps exact offset on the original content`() { + val m = message(MessageRole.USER, "Shout LOUDLY: DePlOy!") + + assertEquals(listOf(SearchMatch(0, 14)), hits(controller.findMatches(listOf(m), "deploy"))) + } + + @Test + fun `caps matches at MAX_SEARCH_MATCHES but keeps the exact total`() { + val content = "a ".repeat(2_000) + val m = message(MessageRole.USER, content) + + val result = controller.findMatches(listOf(m), "a") + assertEquals(ChatSearchController.MAX_SEARCH_MATCHES, result.matches.size) + assertEquals(2_000, result.totalMatches) + assertTrue(result.capped) + // First and last stored entries are real hits with valid offsets. + assertEquals(0, result.matches.first().contentOffset) + assertTrue(result.matches.last().contentOffset >= 0) + } + + @Test + fun `caps across multiple messages counting the exact total`() { + val messages = + List(300) { message(MessageRole.USER, "x ") } + // 300 × 1 hit = 300 + List(300) { message(MessageRole.USER, "x x") } // 300 × 2 hits = 600 + + val result = controller.findMatches(messages, "x") + assertEquals(ChatSearchController.MAX_SEARCH_MATCHES, result.matches.size) + assertEquals(900, result.totalMatches) + assertTrue(result.capped) + } + + @Test + fun `navigate wraps forward and backward`() { + assertEquals(1, controller.navigate(currentIndex = 0, matchCount = 3, direction = 1)) + assertEquals(0, controller.navigate(currentIndex = 2, matchCount = 3, direction = 1)) + assertEquals(2, controller.navigate(currentIndex = 0, matchCount = 3, direction = -1)) + assertEquals(1, controller.navigate(currentIndex = 2, matchCount = 3, direction = -1)) + assertEquals(-1, controller.navigate(currentIndex = -1, matchCount = 0, direction = 1)) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegateTest.kt new file mode 100644 index 00000000..a72289c0 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegateTest.kt @@ -0,0 +1,171 @@ +package com.m57.hermescontrol.ui.chat + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class ChatSearchDelegateTest { + private fun stateWith(vararg contents: String): MutableStateFlow = + MutableStateFlow( + ChatUiState( + messages = + contents.mapIndexed { index, content -> + ChatMessage( + id = "m$index", + role = MessageRole.USER, + content = content, + ) + }, + ), + ) + + @Test + fun `typing pauses coalesce into a single search for the final query`() = + runTest { + val uiState = stateWith("alpha beta", "nothing here") + val delegate = + ChatSearchDelegate( + scope = backgroundScope, + uiState = uiState, + dispatcher = StandardTestDispatcher(testScheduler), + debounceMs = 150, + ) + + delegate.setSearchQuery("alp") + advanceTimeBy(50) + delegate.setSearchQuery("al") + advanceTimeBy(50) + delegate.setSearchQuery("alpha") + + // Debounce window not closed yet — no scan has run. + advanceTimeBy(149) + runCurrent() + assertEquals(emptyList(), delegate.searchState.matchIndices) + + // Typing paused long enough — exactly one scan, for the final query. + advanceTimeBy(1) + runCurrent() + assertEquals(listOf(0), delegate.searchState.matchIndices) + assertEquals(listOf(0), delegate.searchState.matchOffsets) + assertEquals(1, delegate.searchState.matchTotal) + assertFalse(delegate.searchState.matchCapped) + assertEquals("alpha", delegate.searchState.query) + assertEquals(setOf("m0"), delegate.searchState.matchedIds) + assertEquals("m0", delegate.searchState.currentMatchId) + assertEquals(0, delegate.searchState.currentIndex) + } + + @Test + fun `cancelled intermediate queries never produce matches`() = + runTest { + val uiState = stateWith("alpha one", "beta two") + val delegate = + ChatSearchDelegate( + scope = backgroundScope, + uiState = uiState, + dispatcher = StandardTestDispatcher(testScheduler), + debounceMs = 150, + ) + + delegate.setSearchQuery("alpha") + advanceTimeBy(100) + // New keystroke cancels the pending "alpha" scan before it fires. + delegate.setSearchQuery("beta") + advanceTimeBy(150) + runCurrent() + + // Only "beta" ever ran — a stale "alpha" scan must not land. + assertEquals(listOf(1), delegate.searchState.matchIndices) + assertEquals("beta", delegate.searchState.query) + assertEquals("m1", delegate.searchState.currentMatchId) + } + + @Test + fun `blank query clears matches immediately without waiting for debounce`() = + runTest { + val uiState = stateWith("alpha one") + val delegate = + ChatSearchDelegate( + scope = backgroundScope, + uiState = uiState, + dispatcher = StandardTestDispatcher(testScheduler), + debounceMs = 150, + ) + + delegate.setSearchQuery("alpha") + advanceTimeBy(150) + runCurrent() + assertEquals(listOf(0), delegate.searchState.matchIndices) + + delegate.setSearchQuery("") + assertEquals(emptyList(), delegate.searchState.matchIndices) + assertEquals(-1, delegate.searchState.currentIndex) + assertNull(delegate.searchState.currentMatchId) + assertEquals("", delegate.searchState.query) + } + + @Test + fun `navigation updates currentIndex and the current match id`() = + runTest { + val uiState = stateWith("alpha alpha alpha") + val delegate = + ChatSearchDelegate( + scope = backgroundScope, + uiState = uiState, + dispatcher = StandardTestDispatcher(testScheduler), + debounceMs = 150, + ) + + delegate.setSearchQuery("alpha") + advanceTimeBy(150) + runCurrent() + assertEquals(3, delegate.searchState.matchIndices.size) + assertEquals("m0", delegate.searchState.currentMatchId) + + delegate.navigateSearchMatch(1) + assertEquals(1, delegate.searchState.currentIndex) + assertEquals("m0", delegate.searchState.currentMatchId) + + delegate.navigateSearchMatch(-1) + assertEquals(0, delegate.searchState.currentIndex) + } + + @Test + fun `clearSearch resets everything including matched sets`() = + runTest { + val uiState = stateWith("alpha one") + val delegate = + ChatSearchDelegate( + scope = backgroundScope, + uiState = uiState, + dispatcher = StandardTestDispatcher(testScheduler), + debounceMs = 150, + ) + + delegate.toggleSearch() + delegate.setSearchQuery("alpha") + advanceTimeBy(150) + runCurrent() + assertTrue(delegate.searchState.isActive) + assertEquals(setOf("m0"), delegate.searchState.matchedIds) + + delegate.clearSearch() + assertFalse(delegate.searchState.isActive) + assertEquals("", delegate.searchState.query) + assertEquals(emptyList(), delegate.searchState.matchIndices) + assertEquals(0, delegate.searchState.matchTotal) + assertFalse(delegate.searchState.matchCapped) + assertEquals(-1, delegate.searchState.currentIndex) + assertEquals(emptySet(), delegate.searchState.matchedIds) + assertNull(delegate.searchState.currentMatchId) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchPerfSanityTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchPerfSanityTest.kt new file mode 100644 index 00000000..e3af9174 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatSearchPerfSanityTest.kt @@ -0,0 +1,91 @@ +package com.m57.hermescontrol.ui.chat + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * End-to-end sanity on a synthetic LONG session (2000 messages, ~1 KB each). + * + * The matcher must stay correct at scale: exact occurrence counts, valid + * offsets, cap semantics. No wall-clock asserts (flaky on CI) — the local + * scan duration is measured manually during development. + */ +class ChatSearchPerfSanityTest { + private fun longSession(): List = + (0 until 2_000).map { index -> + val role = if (index % 2 == 0) MessageRole.USER else MessageRole.ASSISTANT + val words = + buildList { + repeat(80) { add(listOf("alpha", "beta", "gamma", "deploy", "config")[it % 5]) } + add("needle-$index") + } + ChatMessage( + id = "m$index", + role = role, + content = words.joinToString(" "), + ) + } + + @Test + fun `common word across a long session reports the exact total and caps storage`() { + val messages = longSession() + val result = ChatSearchController().findMatches(messages, "the") + // "the" never appears (content is alpha/beta/...), so no matches. + assertEquals(0, result.totalMatches) + assertEquals(0, result.matches.size) + assertFalse(result.capped) + } + + @Test + fun `long session search returns exact counts for a frequent word`() { + val messages = longSession() + val result = ChatSearchController().findMatches(messages, "deploy") + + // "deploy" is the 4th of 5 rotating words → appears 16× per message, + // 2000 messages → 32_000 total occurrences. + assertEquals(32_000, result.totalMatches) + assertTrue(result.capped) + assertEquals(ChatSearchController.MAX_SEARCH_MATCHES, result.matches.size) + + // Every stored offset points at an actual "deploy" in its message. + for (match in result.matches) { + val message = messages[match.messageIndex] + val start = match.contentOffset + assertTrue(start in 0..message.content.length - 6) + assertTrue( + message.content.substring(start, start + 6).equals("deploy", ignoreCase = true), + ) + } + } + + @Test + fun `long session search finds a unique word at scale`() { + val messages = longSession() + val result = ChatSearchController().findMatches(messages, "needle-1999") + + assertEquals(1, result.totalMatches) + assertEquals(1, result.matches.size) + assertFalse(result.capped) + val match = result.matches.single() + assertEquals(1999, match.messageIndex) + assertTrue(match.contentOffset >= 0) + } + + @Test + fun `tool-heavy long session excludes payload noise`() { + val messages = + longSession().mapIndexed { index, m -> + if (index % 10 == 0) { + m.copy(role = MessageRole.TOOL, content = "tool payload containing needle-1999 repeated") + } else { + m + } + } + // The only real visible hit is the assistant message's own needle. + val result = ChatSearchController().findMatches(messages, "needle-1999") + assertEquals(1, result.totalMatches) + assertEquals(1999, result.matches.single().messageIndex) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index 65807b67..e86717f3 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -2731,12 +2731,12 @@ class ChatViewModelTest { val viewModel = createViewModel() advanceUntilIdle() - assertFalse(viewModel.uiState.value.isSearchActive) + assertFalse(viewModel.searchState.isActive) viewModel.toggleSearch() advanceUntilIdle() - assertTrue(viewModel.uiState.value.isSearchActive) + assertTrue(viewModel.searchState.isActive) viewModel.setSearchQuery("test") advanceUntilIdle() @@ -2744,8 +2744,8 @@ class ChatViewModelTest { viewModel.toggleSearch() advanceUntilIdle() - assertFalse(viewModel.uiState.value.isSearchActive) - assertEquals("", viewModel.uiState.value.searchQuery) + assertFalse(viewModel.searchState.isActive) + assertEquals("", viewModel.searchState.query) } @Test diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt index ab42c130..28595334 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/FullBleedTurnsTest.kt @@ -2,9 +2,13 @@ package com.m57.hermescontrol.ui.chat import com.m57.hermescontrol.ui.chat.fullbleed.AgentEntry import com.m57.hermescontrol.ui.chat.fullbleed.ChatTurn +import com.m57.hermescontrol.ui.chat.fullbleed.currentMatchMessageId import com.m57.hermescontrol.ui.chat.fullbleed.groupIntoTurns import com.m57.hermescontrol.ui.chat.fullbleed.groupIntoTurnsWithStreaming +import com.m57.hermescontrol.ui.chat.fullbleed.matchedMessageIds +import com.m57.hermescontrol.ui.chat.fullbleed.messageIdToLazyIndex import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Test class FullBleedTurnsTest { @@ -145,4 +149,63 @@ class FullBleedTurnsTest { val s = msg("s1", MessageRole.ASSISTANT, content = "partial", isStreaming = true) assertEquals(listOf(entries(AgentEntry.Prose(s))), groupIntoTurnsWithStreaming(emptyList(), s)) } + + // ── messageIdToLazyIndex ─────────────────────────────────────────────── + + @Test + fun `lazy index maps simple user turns one to one`() { + val u1 = msg("u1", MessageRole.USER) + val u2 = msg("u2", MessageRole.USER) + val map = messageIdToLazyIndex(groupIntoTurns(listOf(u1, u2))) + assertEquals(2, map.size) + assertEquals(0, map["u1"]) + assertEquals(1, map["u2"]) + } + + @Test + fun `lazy index accounts for the reasoning hoist item`() { + val a1 = ChatMessage(id = "a1", role = MessageRole.ASSISTANT, content = "prose", reasoningText = "thinking") + val t1 = msg("t1", MessageRole.TOOL) + val a2 = msg("a2", MessageRole.ASSISTANT) + val map = messageIdToLazyIndex(groupIntoTurns(listOf(a1, t1, a2))) + // reasoning-a1 item at lazy 0, prose a1 at 1, tool t1 at 2, prose a2 at 3 + assertEquals(1, map["a1"]) + assertEquals(3, map["a2"]) + // tool rows are items but never map (not searchable). + assertNull(map["t1"]) + } + + @Test + fun `lazy index offsets by leading items`() { + val u1 = msg("u1", MessageRole.USER) + val u2 = msg("u2", MessageRole.USER) + // loading-older spinner occupies lazy item 0. + val map = messageIdToLazyIndex(groupIntoTurns(listOf(u1, u2)), leadingItems = 1) + assertEquals(1, map["u1"]) + assertEquals(2, map["u2"]) + } + + // ── currentMatchMessageId / matchedMessageIds ───────────────────────── + + @Test + fun `currentMatchMessageId resolves once for the current index`() { + val u1 = msg("u1", MessageRole.USER) + val a1 = msg("a1", MessageRole.ASSISTANT) + val msgs = listOf(u1, a1) + assertEquals("a1", currentMatchMessageId(msgs, listOf(0, 1), 1)) + assertEquals("u1", currentMatchMessageId(msgs, listOf(0, 1), 0)) + assertNull(currentMatchMessageId(msgs, listOf(0, 1), -1)) + assertNull(currentMatchMessageId(msgs, emptyList(), 0)) + assertNull(currentMatchMessageId(msgs, listOf(9), 0)) + } + + @Test + fun `matchedMessageIds returns unique ids for match indices`() { + val u1 = msg("u1", MessageRole.USER) + val a1 = msg("a1", MessageRole.ASSISTANT) + val a2 = msg("a2", MessageRole.ASSISTANT) + val msgs = listOf(u1, a1, a2) + assertEquals(setOf("u1", "a2"), matchedMessageIds(msgs, listOf(0, 2, 0))) + assertEquals(emptySet(), matchedMessageIds(msgs, listOf(9))) + } }