From 3aa40414e6b2cd1dfe711a4dc845581a60bfbb4b Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Tue, 7 Jul 2026 23:55:28 +0300 Subject: [PATCH] refactor(chat): extract ChatSearchDelegate from ChatViewModel (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(chat): extract ChatSearchDelegate from ChatViewModel god-object Move the 4 search methods (toggleSearch/setSearchQuery/navigateSearchMatch/ clearSearch) + searchJob field into a dedicated ChatSearchDelegate that owns the chat-search state transitions against the shared _uiState flow. ChatViewModel keeps thin one-line delegating wrappers so its public API (used by ChatScreen) is unchanged. This is the first separable slice of the ChatViewModel god-object (1421 LOC, 47 funcs). Streaming/session/WS clusters are intentionally left in place - they are entangled and will be addressed separately to avoid splitting one logical op across classes. Verified: ktlintCheck PASS, compileDebugKotlin PASS (local SDK). * refactor(chat): extract ChatAttachmentsDelegate from ChatViewModel Move addAttachment/removeAttachment/clearAttachments into a dedicated ChatAttachmentsDelegate that owns pendingAttachments state transitions against the shared _uiState flow. ChatViewModel keeps 3 one-line delegating wrappers - public API unchanged. Read sites (sendMessage) already read pendingAttachments straight off uiState, so no delegate read API needed. Second separable slice of the ChatViewModel god-object (after #486 search). Verified: ktlintCheck PASS, compileDebugKotlin PASS (local SDK). * refactor(chat): address Sourcery review on search extraction - Inject the CoroutineDispatcher into ChatSearchDelegate instead of hardcoding Dispatchers.Default (defaults to Dispatchers.Default so behavior is unchanged; now customizable per environment). - Group the four search delegating methods in ChatViewModel under a 'Compatibility façade' comment marking them as the transitional API surface around ChatSearchDelegate. --- .../ui/chat/ChatAttachmentsDelegate.kt | 52 ++++++++ .../ui/chat/ChatSearchDelegate.kt | 103 ++++++++++++++++ .../hermescontrol/ui/chat/ChatViewModel.kt | 115 +++--------------- 3 files changed, 172 insertions(+), 98 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentsDelegate.kt create mode 100644 app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentsDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentsDelegate.kt new file mode 100644 index 00000000..8d0adcb4 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatAttachmentsDelegate.kt @@ -0,0 +1,52 @@ +package com.m57.hermescontrol.ui.chat + +import com.m57.hermescontrol.data.model.Attachment +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.update + +/** + * Owns pending-attachment state for [ChatViewModel], extracted to keep the + * god-object focused on messaging/session/streaming concerns. + * + * Behavior is identical to the original inline implementation: the three + * methods only mutate `_uiState.pendingAttachments`. Read sites (e.g. sendMessage) + * continue to read `pendingAttachments` straight off the shared uiState flow. + */ +class ChatAttachmentsDelegate( + private val uiState: MutableStateFlow, +) { + fun addAttachment( + uri: String, + name: String, + mimeType: String, + size: Long, + ) { + uiState.update { state -> + val attachment = + Attachment( + uri = uri, + name = name, + mimeType = mimeType, + size = size, + ) + state.copy( + pendingAttachments = state.pendingAttachments + attachment, + ) + } + } + + fun removeAttachment(index: Int) { + uiState.update { state -> + state.copy( + pendingAttachments = + state.pendingAttachments.toMutableList().apply { + if (index in indices) removeAt(index) + }, + ) + } + } + + fun clearAttachments() { + uiState.update { it.copy(pendingAttachments = emptyList()) } + } +} 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 new file mode 100644 index 00000000..decf0463 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatSearchDelegate.kt @@ -0,0 +1,103 @@ +package com.m57.hermescontrol.ui.chat + +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +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. + * + * @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. + */ +class ChatSearchDelegate( + private val scope: CoroutineScope, + private val uiState: MutableStateFlow, + private val searchController: ChatSearchController = ChatSearchController(), + private val dispatcher: CoroutineDispatcher = Dispatchers.Default, +) { + private var searchJob: Job? = null + + fun toggleSearch() { + val current = uiState.value + if (current.isSearchActive) { + clearSearch() + } else { + uiState.update { it.copy(isSearchActive = true, searchQuery = "") } + } + } + + fun setSearchQuery(query: String) { + searchJob?.cancel() + + if (query.isBlank()) { + uiState.update { + it.copy( + searchQuery = query, + searchMatchIndices = emptyList(), + currentSearchMatchIndex = -1, + ) + } + return + } + + // Keep local state in sync immediately so UI feels responsive. + uiState.update { + it.copy(searchQuery = query) + } + + searchJob = scope.launch(dispatcher) { runSearch(query) } + } + + private fun runSearch(query: String) { + val messages = uiState.value.messages + val indices = 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 + } + } + } + + 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) + } + } + + fun clearSearch() { + uiState.update { + it.copy( + isSearchActive = false, + searchQuery = "", + searchMatchIndices = emptyList(), + currentSearchMatchIndex = -1, + ) + } + } +} 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 8c069c4b..ec68b068 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 @@ -113,7 +113,12 @@ class ChatViewModel( private val pendingRequests = ConcurrentHashMap() private val slashDispatcher = SlashCommandDispatcher() - private val searchController = ChatSearchController() + private val searchDelegate = + ChatSearchDelegate( + scope = viewModelScope, + uiState = _uiState, + ) + private val attachmentsDelegate = ChatAttachmentsDelegate(uiState = _uiState) /** Tracks the ID of the currently streaming assistant message. */ @Volatile @@ -742,35 +747,11 @@ class ChatViewModel( name: String, mimeType: String, size: Long, - ) { - _uiState.update { state -> - val attachment = - Attachment( - uri = uri, - name = name, - mimeType = mimeType, - size = size, - ) - state.copy( - pendingAttachments = state.pendingAttachments + attachment, - ) - } - } + ) = attachmentsDelegate.addAttachment(uri, name, mimeType, size) - fun removeAttachment(index: Int) { - _uiState.update { state -> - state.copy( - pendingAttachments = - state.pendingAttachments.toMutableList().apply { - if (index in indices) removeAt(index) - }, - ) - } - } + fun removeAttachment(index: Int) = attachmentsDelegate.removeAttachment(index) - fun clearAttachments() { - _uiState.update { it.copy(pendingAttachments = emptyList()) } - } + fun clearAttachments() = attachmentsDelegate.clearAttachments() private fun handleSlashCommand(command: String) { val userMsg = ChatMessage(role = MessageRole.USER, content = command) @@ -1230,80 +1211,18 @@ class ChatViewModel( } // ── Search ──────────────────────────────────────────────────────────── + // Compatibility façade: stable public API around ChatSearchDelegate. + // These thin delegates keep ChatViewModel's public surface intact while + // the search logic now lives in the delegate. Safe to remove once all + // callers migrate directly to the delegate. - fun toggleSearch() { - val current = _uiState.value - if (current.isSearchActive) { - clearSearch() - } else { - _uiState.update { it.copy(isSearchActive = true, searchQuery = "") } - } - } + fun toggleSearch() = searchDelegate.toggleSearch() - private var searchJob: Job? = null + fun setSearchQuery(query: String) = searchDelegate.setSearchQuery(query) - fun setSearchQuery(query: String) { - searchJob?.cancel() + fun navigateSearchMatch(direction: Int) = searchDelegate.navigateSearchMatch(direction) - if (query.isBlank()) { - _uiState.update { - it.copy( - searchQuery = query, - searchMatchIndices = emptyList(), - currentSearchMatchIndex = -1, - ) - } - return - } - - // Keep local state in sync immediately so UI feels responsive. - _uiState.update { - it.copy(searchQuery = query) - } - - searchJob = - viewModelScope.launch(Dispatchers.Default) { - val messages = _uiState.value.messages - val indices = 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 - } - } - } - } - - 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) - } - } - - fun clearSearch() { - _uiState.update { - it.copy( - isSearchActive = false, - searchQuery = "", - searchMatchIndices = emptyList(), - currentSearchMatchIndex = -1, - ) - } - } + fun clearSearch() = searchDelegate.clearSearch() private var isTestEnv: Boolean? = null