Skip to content

Commit fafa32f

Browse files
Addressing comments
1 parent f4928f6 commit fafa32f

4 files changed

Lines changed: 100 additions & 12 deletions

File tree

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/models/DuckAiModelManager.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ interface DuckAiModelManager {
6565

6666
suspend fun selectReasoningMode(mode: ReasoningMode)
6767

68-
fun setChatScopedReasoningMode(mode: ReasoningMode?)
68+
suspend fun setChatScopedReasoningMode(mode: ReasoningMode?)
6969

7070
fun getSelectedModelId(): String?
7171

@@ -216,8 +216,10 @@ class RealDuckAiModelManager @Inject constructor(
216216
}
217217
}
218218

219-
override fun setChatScopedReasoningMode(mode: ReasoningMode?) {
220-
_modelState.value = _modelState.value.copy(chatScopedReasoningMode = mode)
219+
override suspend fun setChatScopedReasoningMode(mode: ReasoningMode?) {
220+
stateMutex.withLock {
221+
_modelState.value = _modelState.value.copy(chatScopedReasoningMode = mode)
222+
}
221223
}
222224

223225
override fun getSelectedModelId(): String? = _modelState.value.selectedModelId

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModel.kt

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@ class NativeInputModeWidgetViewModel @Inject constructor(
129129
private val currentChat = MutableStateFlow<DuckAiChat?>(null)
130130
private var currentChatJob: Job? = null
131131

132+
// Defensive buffer for the (rare) case where setActiveChatId fires before configure has set
133+
// activeTabId. Replayed inside configure / configureContextual when activeTabId becomes known.
134+
private var pendingChatId: String? = null
135+
private var hasPendingChatId = false
136+
132137
private val commandChannel = Channel<Command>(capacity = 1, onBufferOverflow = DROP_OLDEST)
133138
val commands = commandChannel.receiveAsFlow()
134139

@@ -157,8 +162,12 @@ class NativeInputModeWidgetViewModel @Inject constructor(
157162
}
158163

159164
fun getSelectedModelId(): String? {
160-
// Existing chat: send chat's stored model.
161-
validChat()?.model?.let { return it }
165+
// Existing chat: send the chat's stored model — but only if its model is still in the
166+
// current list. If not (server stopped offering it, slug renamed, etc.) use global instead.
167+
val chat = validChat()
168+
if (chat != null && ReasoningResolver.forChat(chat, modelManager.modelState.value) != null) {
169+
return chat.model
170+
}
162171
// New chat with picker off: nothing to send.
163172
if (!_modelPickerEnabled.value) return null
164173
// New chat with picker on: send what the user picked.
@@ -273,15 +282,23 @@ class NativeInputModeWidgetViewModel @Inject constructor(
273282
}
274283

275284
fun setActiveChatId(chatId: String?) {
276-
val tabId = activeTabId.value ?: return
285+
val tabId = activeTabId.value
286+
if (tabId == null) {
287+
// configure hasn't run yet — buffer until activeTabId is known, replayed in configure.
288+
pendingChatId = chatId
289+
hasPendingChatId = true
290+
return
291+
}
292+
applyChatId(tabId, chatId)
293+
}
294+
295+
private fun applyChatId(tabId: String, chatId: String?) {
277296
nativeInputStatePublisher.update(tabId) { it.copy(chatId = chatId) }
278-
modelManager.setChatScopedReasoningMode(null)
279297
currentChatJob?.cancel()
280298
currentChat.value = null
281-
if (chatId != null) {
282-
currentChatJob = viewModelScope.launch {
283-
currentChat.value = duckAiChatStore.getChatById(chatId)
284-
}
299+
currentChatJob = viewModelScope.launch {
300+
modelManager.setChatScopedReasoningMode(null)
301+
if (chatId != null) currentChat.value = duckAiChatStore.getChatById(chatId)
285302
}
286303
}
287304

@@ -290,6 +307,16 @@ class NativeInputModeWidgetViewModel @Inject constructor(
290307
val context = if (isDuckAiMode) NativeInputState.InputContext.DUCK_AI else NativeInputState.InputContext.BROWSER
291308
val position = if (isBottom) NativeInputState.InputPosition.BOTTOM else NativeInputState.InputPosition.TOP
292309
widgetConfig.value = WidgetConfig(inputContext = context, inputPosition = position)
310+
replayPendingChatId(tabId)
311+
}
312+
313+
private fun replayPendingChatId(tabId: String) {
314+
if (hasPendingChatId) {
315+
val pending = pendingChatId
316+
pendingChatId = null
317+
hasPendingChatId = false
318+
applyChatId(tabId, pending)
319+
}
293320
}
294321

295322
fun storePendingPrompt(
@@ -306,6 +333,7 @@ class NativeInputModeWidgetViewModel @Inject constructor(
306333
fun configureContextual(tabId: String) {
307334
activeTabId.value = tabId
308335
widgetConfig.update { it.copy(inputContext = NativeInputState.InputContext.DUCK_AI_CONTEXTUAL) }
336+
replayPendingChatId(tabId)
309337
}
310338

311339
fun cancelChatSuggestions() {

duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/ReasoningModePickerViewModel.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ class ReasoningModePickerViewModel @Inject constructor(
132132
}
133133
if (match.isAccessible) {
134134
if (chatResolution != null) {
135-
modelManager.setChatScopedReasoningMode(mode)
135+
viewModelScope.launch { modelManager.setChatScopedReasoningMode(mode) }
136136
} else {
137137
viewModelScope.launch { modelManager.selectReasoningMode(mode) }
138138
}

duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/NativeInputModeWidgetViewModelTest.kt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,10 @@ class NativeInputModeWidgetViewModelTest {
558558
@Test
559559
fun whenChatIdSetThenGetSelectedModelIdReturnsChatsModel() = runTest {
560560
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
561+
val modelStateFlow = MutableStateFlow(
562+
ModelState(models = listOf(aiModel(id = "chat-model", supported = listOf(ReasoningEffort.NONE)))),
563+
)
564+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
561565
whenever(duckAiChatStore.getChatById("chat-1")).thenReturn(
562566
DuckAiChat(chatId = "chat-1", title = "t", model = "chat-model", lastEdit = "now", pinned = false),
563567
)
@@ -569,11 +573,37 @@ class NativeInputModeWidgetViewModelTest {
569573
assertEquals("chat-model", viewModel.getSelectedModelId())
570574
}
571575

576+
@Test
577+
fun whenChatModelNotInModelsListThenGetSelectedModelIdFallsBackToGlobal() = runTest {
578+
// Chat references "missing-model" no longer offered server-side. Submission must fall back to
579+
// global on both halves — chat-model + global-effort would be a mismatched pair.
580+
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
581+
val modelStateFlow = MutableStateFlow(
582+
ModelState(
583+
models = listOf(aiModel(id = "other-model", supported = listOf(ReasoningEffort.NONE))),
584+
),
585+
)
586+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
587+
whenever(duckAiChatStore.getChatById("chat-1")).thenReturn(
588+
DuckAiChat(chatId = "chat-1", title = "t", model = "missing-model", lastEdit = "now", pinned = false),
589+
)
590+
val viewModel = createViewModel()
591+
viewModel.configure(tabId = "tab-A", isDuckAiMode = true, isBottom = false)
592+
viewModel.setActiveChatId("chat-1")
593+
advanceUntilIdle()
594+
595+
assertEquals("global-model", viewModel.getSelectedModelId())
596+
}
597+
572598
@Test
573599
fun whenChatIdSetAndPickerDisabledThenGetSelectedModelIdStillReturnsChatsModel() = runTest {
574600
// Production binds modelPickerEnabled to `chatId == null`, so existing chats always have the
575601
// picker disabled. Submission must still carry the chat's stored model.
576602
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
603+
val modelStateFlow = MutableStateFlow(
604+
ModelState(models = listOf(aiModel(id = "chat-model", supported = listOf(ReasoningEffort.NONE)))),
605+
)
606+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
577607
whenever(duckAiChatStore.getChatById("chat-1")).thenReturn(
578608
DuckAiChat(chatId = "chat-1", title = "t", model = "chat-model", lastEdit = "now", pinned = false),
579609
)
@@ -603,6 +633,15 @@ class NativeInputModeWidgetViewModelTest {
603633
// for chat-B is suspended. Submission must fall back to global rather than return chat-A's
604634
// model tagged to chat-B.
605635
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
636+
val modelStateFlow = MutableStateFlow(
637+
ModelState(
638+
models = listOf(
639+
aiModel(id = "chat-A-model", supported = listOf(ReasoningEffort.NONE)),
640+
aiModel(id = "chat-B-model", supported = listOf(ReasoningEffort.NONE)),
641+
),
642+
),
643+
)
644+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
606645
whenever(duckAiChatStore.getChatById("chat-A")).thenReturn(
607646
DuckAiChat(chatId = "chat-A", title = "t", model = "chat-A-model", lastEdit = "now", pinned = false),
608647
)
@@ -634,6 +673,10 @@ class NativeInputModeWidgetViewModelTest {
634673
// setActiveChatId nulls currentChat synchronously and launches the lookup. During the in-
635674
// flight window, submission must fall back to global rather than return chat-A's data.
636675
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
676+
val modelStateFlow = MutableStateFlow(
677+
ModelState(models = listOf(aiModel(id = "chat-A-model", supported = listOf(ReasoningEffort.NONE)))),
678+
)
679+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
637680
whenever(duckAiChatStore.getChatById("chat-A")).thenReturn(
638681
DuckAiChat(chatId = "chat-A", title = "t", model = "chat-A-model", lastEdit = "now", pinned = false),
639682
)
@@ -1092,6 +1135,21 @@ class NativeInputModeWidgetViewModelTest {
10921135
assertNull(nativeInputStateProvider.stateForTab(tabId).value.chatId)
10931136
}
10941137

1138+
@Test
1139+
fun whenSetActiveChatIdBeforeConfigureThenChatIdIsBufferedAndPublishedOnConfigure() = runTest {
1140+
// Defensive: setActiveChatId fires before activeTabId is set. The pending value must be
1141+
// replayed once configure runs.
1142+
val freshViewModel = createViewModel()
1143+
1144+
freshViewModel.setActiveChatId("chat-123")
1145+
advanceUntilIdle()
1146+
1147+
freshViewModel.configure(tabId = "tab-A", isDuckAiMode = false, isBottom = false)
1148+
advanceUntilIdle()
1149+
1150+
assertEquals("chat-123", nativeInputStateProvider.stateForTab("tab-A").value.chatId)
1151+
}
1152+
10951153
// endregion
10961154

10971155
// region fireChatHistorySelectedPixel

0 commit comments

Comments
 (0)