Surface rename failure as a Boolean instead of an exception

ChatHistoryRepository.renameChat now returns a Boolean: true when the
store applied the rename, false when it could not (chat missing, JSON
malformed). The ViewModel branches on that flag and emits the same
Error outcome for both the false path and any thrown exception — the
fragment shows one generic snackbar either way. Drops the check{}
that turned a soft failure into IllegalStateException and removes the
unused throwable field on RenameResult.Error.

Author: GerardPaligot

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/ChatHistoryRepository.kt

@@ -35,7 +35,7 @@ interface ChatHistoryRepository {
 
     suspend fun deleteChat(chatId: String)
     suspend fun deleteAllChats()
-    suspend fun renameChat(chatId: String, newTitle: String)
+    suspend fun renameChat(chatId: String, newTitle: String): Boolean
 }
 
 @ContributesBinding(AppScope::class)
@@ -60,11 +60,8 @@ class RealChatHistoryRepository @Inject constructor(
         withContext(dispatchers.io()) { chatStore.deleteAllChats() }
     }
 
-    override suspend fun renameChat(chatId: String, newTitle: String) {
-        withContext(dispatchers.io()) {
-            check(chatStore.renameChat(chatId, newTitle)) { "Chat $chatId not found or unreadable" }
-        }
-    }
+    override suspend fun renameChat(chatId: String, newTitle: String): Boolean =
+        withContext(dispatchers.io()) { chatStore.renameChat(chatId, newTitle) }
 
     private fun toChatHistoryItem(chat: DuckAiChat): ChatHistoryItem = ChatHistoryItem(
         chatId = chat.chatId,

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/RenameChatFragment.kt

@@ -85,7 +85,7 @@ class RenameChatFragment : DuckDuckGoFragment(R.layout.fragment_rename_chat) {
     private fun onRenameResult(result: RenameChatViewModel.RenameResult) {
         when (result) {
             RenameChatViewModel.RenameResult.Success -> dismiss()
-            is RenameChatViewModel.RenameResult.Error ->
+            RenameChatViewModel.RenameResult.Error ->
                 Snackbar.make(binding.root, R.string.duck_ai_chat_history_rename_error, Snackbar.LENGTH_SHORT).show()
         }
     }

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/history/RenameChatViewModel.kt

@@ -40,19 +40,19 @@ class RenameChatViewModel @Inject constructor(
 
     fun onSaveClicked(chatId: String, newTitle: String) {
         appScope.launch {
-            try {
-                chatHistoryRepository.renameChat(chatId, newTitle.trim())
-                resultChannel.trySend(RenameResult.Success)
+            val outcome = try {
+                if (chatHistoryRepository.renameChat(chatId, newTitle.trim())) RenameResult.Success else RenameResult.Error
             } catch (e: CancellationException) {
                 throw e
             } catch (e: Exception) {
-                resultChannel.trySend(RenameResult.Error(e))
+                RenameResult.Error
             }
+            resultChannel.trySend(outcome)
         }
     }
 
     sealed interface RenameResult {
         data object Success : RenameResult
-        data class Error(val throwable: Throwable) : RenameResult
+        data object Error : RenameResult
     }
 }

duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryRepositoryTest.kt

@@ -25,6 +25,7 @@ import com.duckduckgo.duckchat.store.impl.DuckAiChatStore
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
@@ -166,19 +167,22 @@ class ChatHistoryRepositoryTest {
     }
 
     @Test
-    fun `renameChat delegates to store on success`() = runTest {
+    fun `renameChat delegates to store and returns true on success`() = runTest {
         whenever(chatStore.renameChat("abc", "New")).thenReturn(true)
 
-        repository.renameChat("abc", "New")
+        val result = repository.renameChat("abc", "New")
 
+        assertTrue(result)
         verify(chatStore).renameChat("abc", "New")
     }
 
-    @Test(expected = IllegalStateException::class)
-    fun `renameChat throws when store reports the chat could not be updated`() = runTest {
+    @Test
+    fun `renameChat returns false when store reports the chat could not be updated`() = runTest {
         whenever(chatStore.renameChat("missing", "New")).thenReturn(false)
 
-        repository.renameChat("missing", "New")
+        val result = repository.renameChat("missing", "New")
+
+        assertFalse(result)
     }
 
     private fun chat(

duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/ChatHistoryViewModelTest.kt

@@ -866,8 +866,9 @@ private class FakeChatHistoryRepository(
         source.value = emptyList()
     }
 
-    override suspend fun renameChat(chatId: String, newTitle: String) {
+    override suspend fun renameChat(chatId: String, newTitle: String): Boolean {
         renamedChats += chatId to newTitle
+        return true
     }
 }
 

duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/history/RenameChatViewModelTest.kt

@@ -22,7 +22,6 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
 
@@ -44,32 +43,42 @@ class RenameChatViewModelTest {
         }
     }
 
+    @Test
+    fun `onSaveClicked emits Error when repository reports failure`() = coroutineRule.testScope.runTest {
+        repository.nextResult = false
+
+        viewModel.results.test {
+            viewModel.onSaveClicked("chat-1", "New Title")
+
+            assertEquals(RenameChatViewModel.RenameResult.Error, awaitItem())
+        }
+    }
+
     @Test
     fun `onSaveClicked emits Error when repository throws`() = coroutineRule.testScope.runTest {
-        val failure = IllegalStateException("boom")
-        repository.errorToThrow = failure
+        repository.errorToThrow = IllegalStateException("boom")
 
         viewModel.results.test {
             viewModel.onSaveClicked("chat-1", "New Title")
 
-            val result = awaitItem()
-            assertTrue(result is RenameChatViewModel.RenameResult.Error)
-            assertEquals(failure, (result as RenameChatViewModel.RenameResult.Error).throwable)
+            assertEquals(RenameChatViewModel.RenameResult.Error, awaitItem())
         }
     }
 }
 
 private class RecordingRenameRepository : ChatHistoryRepository {
     val renames: MutableList<Pair<String, String>> = mutableListOf()
     var errorToThrow: Throwable? = null
+    var nextResult: Boolean = true
 
     override fun observeChats(): Flow<List<ChatHistoryItem>> = MutableStateFlow(emptyList())
 
     override suspend fun deleteChat(chatId: String) = Unit
     override suspend fun deleteAllChats() = Unit
 
-    override suspend fun renameChat(chatId: String, newTitle: String) {
+    override suspend fun renameChat(chatId: String, newTitle: String): Boolean {
         errorToThrow?.let { throw it }
         renames += chatId to newTitle
+        return nextResult
     }
 }