Skip to content

Commit 2882db3

Browse files
YoussefKeyrouzYoussefKeyrouz
committed
Addressing comments
1 parent f4928f6 commit 2882db3

4 files changed

Lines changed: 107 additions & 18 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: 38 additions & 11 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,11 +162,14 @@ class NativeInputModeWidgetViewModel @Inject constructor(
157162
}
158163

159164
fun getSelectedModelId(): String? {
160-
// Existing chat: send chat's stored model.
161-
validChat()?.model?.let { return it }
162-
// New chat with picker off: nothing to send.
163-
if (!_modelPickerEnabled.value) return null
164-
// New chat with picker on: send what the user picked.
165+
// Existing chat with its model still in the list: send the chat's stored model.
166+
val chat = validChat()
167+
if (chat != null && ReasoningResolver.forChat(chat, modelManager.modelState.value) != null) {
168+
return chat.model
169+
}
170+
// Existing chat whose model is no longer in the list: fall through to global so submission
171+
// carries (global model, global effort). New chat with picker off: nothing to send.
172+
if (chat == null && !_modelPickerEnabled.value) return null
165173
return modelManager.getSelectedModelId()
166174
}
167175

@@ -273,15 +281,23 @@ class NativeInputModeWidgetViewModel @Inject constructor(
273281
}
274282

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

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

295321
fun storePendingPrompt(
@@ -306,6 +332,7 @@ class NativeInputModeWidgetViewModel @Inject constructor(
306332
fun configureContextual(tabId: String) {
307333
activeTabId.value = tabId
308334
widgetConfig.update { it.copy(inputContext = NativeInputState.InputContext.DUCK_AI_CONTEXTUAL) }
335+
replayPendingChatId(tabId)
309336
}
310337

311338
fun cancelChatSuggestions() {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ class ReasoningModePickerViewModel @Inject constructor(
102102
nativeState: NativeInputState,
103103
chat: DuckAiChat?,
104104
): ReasoningModePickerState {
105-
val chatMatches = chat?.chatId == nativeState.chatId
106-
val chatResolution = chat?.let { ReasoningResolver.forChat(it, modelState) }
107-
if (nativeState.chatId != null && (!chatMatches || chatResolution == null)) {
105+
val activeChat = chat?.takeIf { it.chatId == nativeState.chatId }
106+
val chatResolution = activeChat?.let { ReasoningResolver.forChat(it, modelState) }
107+
if (nativeState.chatId != null && chatResolution == null) {
108108
return ReasoningModePickerState(visible = false, rows = emptyList(), displayedMode = null)
109109
}
110110
val available = chatResolution?.available ?: modelState.availableReasoningModes
@@ -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: 60 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,39 @@ 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+
// Picker is disabled here to mirror production (host binds modelPickerEnabled to chatId == null).
581+
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
582+
val modelStateFlow = MutableStateFlow(
583+
ModelState(
584+
models = listOf(aiModel(id = "other-model", supported = listOf(ReasoningEffort.NONE))),
585+
),
586+
)
587+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
588+
whenever(duckAiChatStore.getChatById("chat-1")).thenReturn(
589+
DuckAiChat(chatId = "chat-1", title = "t", model = "missing-model", lastEdit = "now", pinned = false),
590+
)
591+
val viewModel = createViewModel()
592+
viewModel.configure(tabId = "tab-A", isDuckAiMode = true, isBottom = false)
593+
viewModel.setActiveChatId("chat-1")
594+
viewModel.setModelPickerEnabled(false)
595+
advanceUntilIdle()
596+
597+
assertEquals("global-model", viewModel.getSelectedModelId())
598+
}
599+
572600
@Test
573601
fun whenChatIdSetAndPickerDisabledThenGetSelectedModelIdStillReturnsChatsModel() = runTest {
574602
// Production binds modelPickerEnabled to `chatId == null`, so existing chats always have the
575603
// picker disabled. Submission must still carry the chat's stored model.
576604
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
605+
val modelStateFlow = MutableStateFlow(
606+
ModelState(models = listOf(aiModel(id = "chat-model", supported = listOf(ReasoningEffort.NONE)))),
607+
)
608+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
577609
whenever(duckAiChatStore.getChatById("chat-1")).thenReturn(
578610
DuckAiChat(chatId = "chat-1", title = "t", model = "chat-model", lastEdit = "now", pinned = false),
579611
)
@@ -603,6 +635,15 @@ class NativeInputModeWidgetViewModelTest {
603635
// for chat-B is suspended. Submission must fall back to global rather than return chat-A's
604636
// model tagged to chat-B.
605637
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
638+
val modelStateFlow = MutableStateFlow(
639+
ModelState(
640+
models = listOf(
641+
aiModel(id = "chat-A-model", supported = listOf(ReasoningEffort.NONE)),
642+
aiModel(id = "chat-B-model", supported = listOf(ReasoningEffort.NONE)),
643+
),
644+
),
645+
)
646+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
606647
whenever(duckAiChatStore.getChatById("chat-A")).thenReturn(
607648
DuckAiChat(chatId = "chat-A", title = "t", model = "chat-A-model", lastEdit = "now", pinned = false),
608649
)
@@ -634,6 +675,10 @@ class NativeInputModeWidgetViewModelTest {
634675
// setActiveChatId nulls currentChat synchronously and launches the lookup. During the in-
635676
// flight window, submission must fall back to global rather than return chat-A's data.
636677
whenever(modelManager.getSelectedModelId()).thenReturn("global-model")
678+
val modelStateFlow = MutableStateFlow(
679+
ModelState(models = listOf(aiModel(id = "chat-A-model", supported = listOf(ReasoningEffort.NONE)))),
680+
)
681+
whenever(modelManager.modelState).thenReturn(modelStateFlow)
637682
whenever(duckAiChatStore.getChatById("chat-A")).thenReturn(
638683
DuckAiChat(chatId = "chat-A", title = "t", model = "chat-A-model", lastEdit = "now", pinned = false),
639684
)
@@ -1092,6 +1137,21 @@ class NativeInputModeWidgetViewModelTest {
10921137
assertNull(nativeInputStateProvider.stateForTab(tabId).value.chatId)
10931138
}
10941139

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

10971157
// region fireChatHistorySelectedPixel

0 commit comments

Comments
 (0)