fix(chat): snappy in-chat search over visible text only (#887)
* 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 <hy4ri@users.noreply.github.com>
This commit is contained in:
1 parent
685b870cad
commit
546bd17b18
16 files changed
+895
-142
No files matched your search
+12
-5
@@ -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<ChatViewModel>(relaxed = true),
|
||||
|
||||
Reference in new issue
Block a user