fix: improve History search and prevent duplicate-key crashes (#1186)

This commit is contained in:
M57 authored and GitHub committed 2026-09-17 23:00:53 +03:00
1 parent 7951acc6eb
commit 9690ce8dc6
9 files changed
+458 -48

No files matched your search

@@ -0,0 +1,62 @@
package com.m57.hermescontrol.ui.sessions
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.m57.hermescontrol.data.model.SessionInfo
import com.m57.hermescontrol.theme.HermesControlTheme
import com.m57.hermescontrol.ui.sessions.components.SearchResultCard
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
@RunWith(AndroidJUnit4::class)
class SearchResultCardTest {
@get:Rule
val compose = createComposeRule()
private var clicks = 0
private fun show(title: String?) {
compose.setContent {
HermesControlTheme {
SearchResultCard(
session = SessionInfo(id = "fixture", title = title, preview = ">>>launch<<< notes excerpt"),
query = "launch notes",
isSelecting = false,
isSelected = false,
isDeleting = false,
highlightBackground = Color.Unspecified,
highlightForeground = Color.Unspecified,
onCardClick = { clicks++ },
onToggleSelection = {},
onSelect = {},
onRename = {},
onDelete = {},
)
}
}
}
@Test
fun titleAndExcerptAreDistinctAndTapOpensResult() {
show("Launch checklist")
compose.onNodeWithTag("search_title_fixture", useUnmergedTree = true).assertIsDisplayed()
compose.onNodeWithText("Launch checklist", useUnmergedTree = true).assertIsDisplayed()
compose.onNodeWithText("launch notes excerpt", useUnmergedTree = true).assertIsDisplayed()
compose.onNodeWithTag("session_card_fixture").performClick()
compose.runOnIdle { assertEquals(1, clicks) }
}
@Test
fun legacyResultDoesNotInventATitle() {
show(null)
compose.onNodeWithTag("search_title_fixture", useUnmergedTree = true).assertDoesNotExist()
compose.onNodeWithText("launch notes excerpt", useUnmergedTree = true).assertIsDisplayed()
}
}
@@ -5,7 +5,11 @@ import kotlinx.serialization.Serializable
@Serializable
data class SessionSearchResponse(
val results: List<SessionSearchResult> = emptyList(),
)
val has_more: Boolean? = null,
val next_offset: Int? = null,
) {
val hasMore: Boolean get() = has_more == true
}
@Serializable
data class SessionSearchResult(
@@ -15,4 +19,9 @@ data class SessionSearchResult(
val source: String? = null,
val model: String? = null,
val session_started: Double? = null,
val title: String? = null,
val last_active: Double? = null,
val message_count: Int? = null,
val archived: Boolean? = null,
val lineage_root: String? = null,
)
@@ -189,6 +189,8 @@ interface HermesApiService : KanbanApiService {
@Query("profile") profile: String? = null,
@Query("source") source: String? = null,
@Query("exclude_sources") excludeSources: String? = null,
@Query("limit") limit: Int = 20,
@Query("offset") offset: Int = 0,
): Response<SessionSearchResponse>
@GET("api/sessions/{id}/messages")
@@ -1,5 +1,11 @@
package com.m57.hermescontrol.ui.sessions
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.SpanStyle
import androidx.compose.ui.text.buildAnnotatedString
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.withStyle
import com.m57.hermescontrol.data.model.SessionInfo
import com.m57.hermescontrol.data.model.SessionSearchResult
import kotlinx.serialization.json.Json
@@ -15,26 +21,27 @@ import java.time.format.FormatStyle
import kotlin.math.abs
/**
* Converts a backend search hit into a display model WITHOUT faking a title.
* The backend returns no session name, only a matched `snippet` + metadata, so the
* card must render the snippet as a match excerpt (never as the title). `title` is
* deliberately left null so [SearchResultCard] shows the honest "Match" layout.
* Converts a backend search hit into a display model.
* When the backend returns an actual session title, it is mapped cleanly.
* When title is missing/null, title is kept null so [SearchResultCard] shows
* the honest fallback "Match" layout.
*/
internal fun SessionSearchResult.toSessionInfo(): SessionInfo =
SessionInfo(
id = session_id,
title = null,
title = title?.takeIf(String::isNotBlank),
preview = snippet,
source = source,
model = model,
started_at = session_started,
// message_count/status aren't in the search payload; leave null so the
// search card hides those normal-list affordances.
last_active = last_active,
message_count = message_count,
lineageRootId = lineage_root,
)
/**
* Formats a backend epoch-seconds timestamp into a friendly, local relative/absolute
* string. Used for search results where the session name is unknown.
* string. Used for search results where the session name is unknown or when displaying recency.
*/
internal fun formatPlayedAt(epochSeconds: Double?): String? {
if (epochSeconds == null || epochSeconds <= 0.0) return null
@@ -96,3 +103,84 @@ private fun JsonElement.searchText(): String? =
?: wrapperFieldNames.firstNotNullOfOrNull { fieldName -> this[fieldName]?.searchText() }
}
}
private val SEARCH_TOKEN_REGEX = Regex("\"([^\"]+)\"|(\\S+)")
/**
* Builds an AnnotatedString highlighting terms from [query] in [text].
* Supports multi-word queries, quoted phrases ("like this"), case-insensitivity,
* overlapping matches, and Unicode characters safely without offset drift.
* Renders as plain text (no HTML interpretation).
*/
fun highlightSearchText(
text: String,
query: String,
highlightBackground: Color,
highlightForeground: Color,
): AnnotatedString {
if (query.isBlank() || text.isEmpty()) {
return AnnotatedString(text)
}
val terms =
SEARCH_TOKEN_REGEX
.findAll(query)
.mapNotNull { match ->
(match.groups[1]?.value ?: match.groups[2]?.value)?.trim()?.takeIf(String::isNotEmpty)
}.distinct()
.toList()
if (terms.isEmpty()) {
return AnnotatedString(text)
}
val ranges = mutableListOf<IntRange>()
for (term in terms) {
var startIndex = 0
while (startIndex < text.length) {
val found = text.indexOf(term, startIndex = startIndex, ignoreCase = true)
if (found == -1) break
ranges.add(found until (found + term.length))
startIndex = found + term.length
}
}
if (ranges.isEmpty()) {
return AnnotatedString(text)
}
ranges.sortBy { it.first }
val merged = mutableListOf<IntRange>()
var current = ranges[0]
for (i in 1 until ranges.size) {
val next = ranges[i]
if (next.first <= current.last + 1) {
current = current.first..maxOf(current.last, next.last)
} else {
merged.add(current)
current = next
}
}
merged.add(current)
return buildAnnotatedString {
var cursor = 0
for (range in merged) {
if (range.first > cursor) {
append(text.substring(cursor, range.first))
}
withStyle(
SpanStyle(
background = highlightBackground,
color = highlightForeground,
fontWeight = FontWeight.Bold,
),
) {
append(text.substring(range.first, range.last + 1))
}
cursor = range.last + 1
}
if (cursor < text.length) {
append(text.substring(cursor))
}
}
}
@@ -661,6 +661,37 @@ fun SessionsScreen(
onDelete = { viewModel.requestDeleteSession(session.id) },
)
}
if (state.searchHasMore) {
item(key = "search_paging_footer") {
Column(horizontalAlignment = Alignment.CenterHorizontally) {
state.searchLoadMoreError?.let { error ->
Text(error, color = MaterialTheme.colorScheme.error)
}
TextButton(
onClick = viewModel::loadMoreSearch,
enabled = !state.isLoadingMoreSearch,
modifier =
Modifier.fillMaxWidth().testTag(
"search_load_more",
),
) {
if (state.isLoadingMoreSearch) {
CircularProgressIndicator(Modifier.size(20.dp))
} else {
Text(
stringResource(
if (state.searchLoadMoreError != null) {
R.string.action_retry
} else {
R.string.history_load_more
},
),
)
}
}
}
}
}
}
}
}
@@ -99,6 +99,10 @@ data class SessionsUiState(
val isSearching: Boolean = false,
val searchResults: List<SessionSearchResult> = emptyList(),
val searchError: String? = null,
val isLoadingMoreSearch: Boolean = false,
val searchHasMore: Boolean = false,
val searchNextOffset: Int? = null,
val searchLoadMoreError: String? = null,
val showHidden: Boolean = false,
val pinnedExpanded: Boolean = true,
val liveStatuses: Map<String, SessionLiveStatus> = emptyMap(),
@@ -255,6 +259,8 @@ class SessionsViewModel(
loadJob?.cancel()
pageJob?.cancel()
searchJob?.cancel()
searchPageJob?.cancel()
searchGeneration++
rawPaginationOffset = 0
val query = _uiState.value.searchQuery
_uiState.update {
@@ -413,6 +419,8 @@ class SessionsViewModel(
// ── Search (server-backed FTS5) ──────────────────────────────────
private var searchJob: Job? = null
private var searchPageJob: Job? = null
private var searchGeneration = 0L
/**
* Debounced server-side session search. A non-blank query schedules a search
@@ -420,53 +428,99 @@ class SessionsViewModel(
* paginated list mode.
*/
fun setSearchQuery(query: String) {
val requestGeneration = generation
val requestGeneration = ++searchGeneration
val section = _uiState.value.section
_uiState.update { it.copy(searchQuery = query, searchResults = emptyList()) }
searchJob?.cancel()
searchPageJob?.cancel()
_uiState.update {
it.copy(
searchQuery = query,
searchResults = emptyList(),
isSearching = query.isNotBlank(),
searchError = null,
isLoadingMoreSearch = false,
searchHasMore = false,
searchNextOffset = null,
searchLoadMoreError = null,
isSelecting = false,
selectedIds = emptySet(),
)
}
if (query.isBlank()) {
_uiState.update {
it.copy(searchResults = emptyList(), searchError = null, isSearching = false)
}
if (_uiState.value.sessions.isEmpty()) loadSessions()
return
}
searchJob =
viewModelScope.launch {
delay(SEARCH_DEBOUNCE_MS)
_uiState.update { it.copy(isSearching = true, searchError = null) }
val result =
safeApiCall {
ApiClient.hermesApi.searchSessions(
q = query,
profile = null,
source = section.source,
excludeSources = section.excludeSources,
)
}
if (requestGeneration != generation || _uiState.value.searchQuery != query) return@launch
when (result) {
is NetworkResult.Success -> {
_uiState.update {
it.copy(
isSearching = false,
searchResults = result.data.results.orEmpty(),
searchError = null,
)
}
}
requestSearchPage(query, section, requestGeneration, 0)
}
}
is NetworkResult.Failure -> {
_uiState.update {
it.copy(
isSearching = false,
searchResults = emptyList(),
searchError = "Search failed: ${result.error.message}",
)
fun loadMoreSearch() {
val state = _uiState.value
val offset = state.searchNextOffset ?: return
if (!state.isSearchMode || state.isSearching || state.isLoadingMoreSearch || !state.searchHasMore) return
val requestGeneration = searchGeneration
_uiState.update { it.copy(isLoadingMoreSearch = true, searchLoadMoreError = null) }
searchPageJob =
viewModelScope.launch {
requestSearchPage(state.searchQuery, state.section, requestGeneration, offset)
}
}
private suspend fun requestSearchPage(
query: String,
section: HistorySection,
requestGeneration: Long,
offset: Int,
) {
val result =
safeApiCall {
ApiClient.hermesApi.searchSessions(
q = query,
profile = null,
source = section.source,
excludeSources = section.excludeSources,
limit = 20,
offset = offset,
)
}
if (requestGeneration != searchGeneration || _uiState.value.section != section) return
when (result) {
is NetworkResult.Success -> {
_uiState.update { state ->
val previous = if (offset == 0) emptyList() else state.searchResults
// Dedup by final surfaced session id: legacy backends can return
// the same continuation twice (distinct lineage roots), and
// LazyList keys session ids — duplicates crash the screen.
val results = (previous + result.data.results).distinctBy { it.session_id }
val next =
result.data.next_offset?.takeIf {
result.data.hasMore && it > offset && results.size > previous.size
}
state.copy(
isSearching = false,
isLoadingMoreSearch = false,
searchResults = results,
searchError = null,
searchLoadMoreError = null,
searchHasMore = next != null,
searchNextOffset = next,
)
}
}
is NetworkResult.Failure -> {
_uiState.update {
if (offset == 0) {
it.copy(isSearching = false, searchError = "Search failed: ${result.error.message}")
} else {
it.copy(isLoadingMoreSearch = false, searchLoadMoreError = "Failed to load more")
}
}
}
}
}
// ── Stats ────────────────────────────────────────────────────────────
@@ -581,6 +635,10 @@ class SessionsViewModel(
it.sessions.map { s ->
if (s.id == sessionId) s.copy(title = newTitle) else s
},
searchResults =
it.searchResults.map { s ->
if (s.session_id == sessionId) s.copy(title = newTitle) else s
},
toastMessage = "Session renamed",
)
}
@@ -780,6 +838,7 @@ class SessionsViewModel(
isDeletingBulk = false,
isSelecting = false,
selectedIds = emptySet(),
searchResults = it.searchResults.filter { hit -> hit.session_id !in ids },
toastMessage = toastMsg,
)
}
@@ -44,11 +44,11 @@ import com.m57.hermescontrol.ui.common.StatusBadge
import com.m57.hermescontrol.ui.common.StatusBadgeType
import com.m57.hermescontrol.ui.sessions.cleanSearchSnippet
import com.m57.hermescontrol.ui.sessions.formatPlayedAt
import com.m57.hermescontrol.ui.sessions.highlightSearchText
/**
* Search-result card. The backend search payload has no session title, so this card is
* honest about it: it shows a "Match" label + the highlighted snippet as the body, plus
* source / model / played-at metadata chips. It never presents the snippet as a name.
* Search-result card with a separate title and matched excerpt. Legacy responses
* without a title retain the Match label; a snippet is never used as a name.
*/
@OptIn(ExperimentalFoundationApi::class, ExperimentalLayoutApi::class)
@Composable
@@ -71,7 +71,7 @@ fun SearchResultCard(
val statusColors = LocalHermesStatusColors.current
val snippet = session.preview?.takeIf { it.isNotBlank() } ?: stringResource(R.string.history_untitled)
val cleanSnippet = cleanSearchSnippet(snippet)
val playedAt = formatPlayedAt(session.started_at)
val playedAt = formatPlayedAt(session.last_active ?: session.started_at)
var menuExpanded by remember { mutableStateOf(false) }
Card(
@@ -145,9 +145,20 @@ fun SearchResultCard(
Spacer(modifier = Modifier.height(spacing.xs))
// The matched snippet, highlighted — shown as the body, NOT as a title.
session.title?.takeIf(String::isNotBlank)?.let { title ->
Text(
text = highlightSearchText(title, query, highlightBackground, highlightForeground),
style = MaterialTheme.typography.titleMedium,
maxLines = 2,
overflow = TextOverflow.Ellipsis,
modifier = Modifier.testTag("search_title_${session.id}"),
)
Spacer(modifier = Modifier.height(spacing.xs))
}
// The matched snippet stays separate from the conversation title.
Text(
text = highlightText(cleanSnippet, query, highlightBackground, highlightForeground),
text = highlightSearchText(cleanSnippet, query, highlightBackground, highlightForeground),
style = MaterialTheme.typography.bodyMedium,
color = MaterialTheme.colorScheme.onSurface,
maxLines = 4,
@@ -7,6 +7,33 @@ import org.junit.Assert.assertNull
import org.junit.Test
class SessionSearchFormattingTest {
@Test
fun `wire title stays distinct from excerpt and preserves metadata`() {
val hit =
kotlinx.serialization.json.Json
.decodeFromString<SessionSearchResult>(
"""{"session_id":"s","title":"Launch","snippet":"match","last_active":42,"message_count":7}""",
).toSessionInfo()
assertEquals("Launch", hit.title)
assertEquals("match", hit.preview)
assertEquals(42.0, hit.last_active)
assertEquals(7, hit.message_count)
}
@Test
fun `highlight phrases and words without Unicode offset drift`() {
val text = "İ 🌙 launch notes and deploy"
val result =
highlightSearchText(
text,
"\"launch notes\" deploy",
androidx.compose.ui.graphics.Color.Transparent,
androidx.compose.ui.graphics.Color.Unspecified,
)
assertEquals(text, result.text)
assertEquals(listOf("launch notes", "deploy"), result.spanStyles.map { text.substring(it.start, it.end) })
}
@Test
fun `clean search snippet strips highlight markers from plain text`() {
assertEquals(
@@ -99,6 +99,66 @@ class SessionsViewModelTest {
unmockkAll()
}
@Test
fun `wire paging metadata drives a second request without changing history paging`() {
val first =
kotlinx.serialization.json.Json.decodeFromString<SessionSearchResponse>(
"""{"results":[{"session_id":"a"},{"session_id":"a"}],"has_more":true,"next_offset":20}""",
)
coEvery { mockApi.searchSessions("launch", null, null, "cron", 20, 0) } returns Response.success(first)
coEvery { mockApi.searchSessions("launch", null, null, "cron", 20, 20) } returns
Response.success(
SessionSearchResponse(listOf(SessionSearchResult("b")), has_more = false),
)
val vm = createViewModel()
vm.setSearchQuery("launch")
testDispatcher.scheduler.advanceUntilIdle()
vm.loadMoreSearch()
testDispatcher.scheduler.advanceUntilIdle()
assertEquals(
listOf("a", "b"),
vm.uiState.value.searchResults
.map { it.session_id },
)
assertFalse(vm.uiState.value.searchHasMore)
assertFalse(vm.uiState.value.hasMore)
coVerify(exactly = 1) { mockApi.searchSessions("launch", null, null, "cron", 20, 20) }
}
@Test
fun `bulk delete removes selected search hits and preserves others`() {
mockkObject(com.m57.hermescontrol.data.local.AuthManager)
every {
com.m57.hermescontrol.data.local.AuthManager
.getLastOpenedSessionId()
} returns null
coEvery { mockApi.searchSessions("launch", null, null, "cron", 20, 0) } returns
Response.success(SessionSearchResponse(listOf(SessionSearchResult("a"), SessionSearchResult("b"))))
coEvery { mockApi.bulkDeleteSessions(any()) } returns
Response.success(
com.m57.hermescontrol.data.model
.BulkDeleteResponse(ok = true, deleted = 1),
)
coEvery { mockApi.getSessions(any(), any(), any(), any(), any()) } returns
Response.success(SessionListResponse(sessions = emptyList(), total = 0))
val vm = createViewModel()
vm.setSearchQuery("launch")
testDispatcher.scheduler.advanceUntilIdle()
vm.toggleSessionSelection("a")
vm.confirmBulkDelete()
testDispatcher.scheduler.advanceUntilIdle()
assertEquals(
listOf("b"),
vm.uiState.value.searchResults
.map { it.session_id },
)
assertTrue(
vm.uiState.value.selectedIds
.isEmpty(),
)
assertFalse(vm.uiState.value.isDeletingBulk)
}
@Test
fun `tracking loads the project list`() {
val projects = FakeProjectsSource().apply { projectsToReturn = listOf(ProjectInfo(id = "p_1", name = "App")) }
@@ -189,6 +249,67 @@ class SessionsViewModelTest {
vm.stopLiveStatusTracking()
}
@Test
fun `search is pending immediately before debounce`() {
val vm = createViewModel()
vm.setSearchQuery("deploy")
assertTrue(vm.uiState.value.isSearching)
assertNull(vm.uiState.value.searchError)
vm.setSearchQuery("")
assertFalse(vm.uiState.value.isSearching)
}
@Test
fun `search pagination keeps results on failure and retries same offset`() {
coEvery { mockApi.searchSessions("deploy", null, null, "cron", 20, 0) } returns
Response.success(SessionSearchResponse(listOf(SessionSearchResult("one")), true, 1))
coEvery { mockApi.searchSessions("deploy", null, null, "cron", 20, 1) } returns
Response.error(503, "busy".toResponseBody()) andThen
Response.error(503, "busy".toResponseBody()) andThen
Response.error(503, "busy".toResponseBody()) andThen
Response.success(
SessionSearchResponse(listOf(SessionSearchResult("one"), SessionSearchResult("two")), false),
)
val vm = createViewModel()
vm.setSearchQuery("deploy")
testDispatcher.scheduler.advanceUntilIdle()
assertTrue(vm.uiState.value.searchHasMore)
vm.loadMoreSearch()
testDispatcher.scheduler.advanceUntilIdle()
assertEquals(
listOf("one"),
vm.uiState.value.searchResults
.map { it.session_id },
)
assertNotNull(vm.uiState.value.searchLoadMoreError)
assertNull(vm.uiState.value.searchError)
vm.loadMoreSearch()
testDispatcher.scheduler.advanceUntilIdle()
assertEquals(
listOf("one", "two"),
vm.uiState.value.searchResults
.map { it.session_id },
)
assertFalse(vm.uiState.value.searchHasMore)
assertFalse(vm.uiState.value.hasMore)
}
@Test
fun `legacy search and non advancing page do not loop`() {
coEvery { mockApi.searchSessions("old", null, null, "cron", 20, 0) } returns
Response.success(SessionSearchResponse(listOf(SessionSearchResult("one"))))
coEvery { mockApi.searchSessions("new", null, null, "cron", 20, 0) } returns
Response.success(SessionSearchResponse(listOf(SessionSearchResult("two")), true, 0))
val vm = createViewModel()
for (query in listOf("old", "new")) {
vm.setSearchQuery(query)
testDispatcher.scheduler.advanceUntilIdle()
assertFalse(vm.uiState.value.searchHasMore)
vm.loadMoreSearch()
}
coVerify(exactly = 2) { mockApi.searchSessions(any(), any(), any(), any(), any(), any()) }
}
@Test
fun `blank query resets search mode`() {
val vm = createViewModel()