Skip to content

Commit 46ad135

Browse files
YoussefKeyrouzYoussefKeyrouz
authored
Per-chat reasoning effort in contextual sheet (#8674)
Task/Issue URL: https://app.asana.com/1/137249556945/project/1212810093780571/task/1215062229903137?focus=true ### Description Wires the contextual Duck.ai sheet into the per-chat reasoning flow introduced by #8666 and fixes two correctness bugs + Minor cleanup in `ReasoningModePickerViewModel` ### Steps to test this PR _Per-chat scope restored on reopen_ - [ ] Start a chat in the contextual sheet on tab A; pick a non-default model and reasoning effort; submit a prompt. - [ ] Close the contextual sheet. - [ ] Open new duck ai on another tab or submit a prompt via the native input field using a new model (global default changes) - [ ] Reopen the contextual sheet on tab A (within the session window). Verify the picker reflects the reasoning effort, not the global default) - [ ] Submit another prompt; confirm that the submission carries the chat's modelId and reasoningEffort. - [ ] Open another tab and start a new chat in the contextual sheet on tab B - [ ] Switching between tabs properly restore the reasoning effort of the corresponding chat. _Globals untouched_ - [ ] After any per-chat submission above, dismiss the sheet and open Duck.ai from the address bar (or a fresh tab). Verify your global defaults are unchanged. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes contextual sheet state management and URL/chat-id tracking, which can affect session restore behavior and what model/reasoning settings get applied to submissions. > > **Overview** > Wires the contextual Duck.ai sheet to a **per-chat** identifier by extracting `chatID` from the webview URL and exposing it as a `StateFlow` on `DuckChatContextualViewModel`. > > The contextual native input widget now receives this `chatIdFlow` (via `ContextualNativeInputManager.init`) and binds it into the native-input state, enabling chat-scoped reasoning/model selection to follow the active chat across sheet reopen/tab switches. The view model centralizes URL/chat-id updates via `setSheetUrl`/`clearSheetUrl` and adds tests to ensure `chatId` is set/cleared correctly; `ReasoningModePickerViewModel` is lightly refactored to use a single snapshot of `modelState`, with tests adjusted accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 666691b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f89c378 commit 46ad135

6 files changed

Lines changed: 108 additions & 12 deletions

File tree

‎duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/ContextualNativeInputManager.kt‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import com.duckduckgo.js.messaging.api.JsMessaging
3030
import com.duckduckgo.js.messaging.api.SubscriptionEventData
3131
import com.google.android.material.card.MaterialCardView
3232
import com.squareup.anvil.annotations.ContributesBinding
33+
import kotlinx.coroutines.flow.Flow
3334
import kotlinx.coroutines.flow.launchIn
3435
import kotlinx.coroutines.flow.onEach
3536
import org.json.JSONArray
@@ -43,6 +44,7 @@ interface ContextualNativeInputManager {
4344
widget: NativeInputModeWidget,
4445
jsMessaging: JsMessaging,
4546
lifecycleOwner: LifecycleOwner,
47+
chatIdFlow: Flow<String?>,
4648
onSearchSubmitted: (String) -> Unit,
4749
onCameraCaptureRequested: (ValueCallback<Array<Uri>>) -> Unit = {},
4850
onFilePickerRequested: (ValueCallback<Array<Uri>>, List<String>) -> Unit = { _, _ -> },
@@ -68,6 +70,7 @@ class RealContextualNativeInputManager @Inject constructor(
6870
widget: NativeInputModeWidget,
6971
jsMessaging: JsMessaging,
7072
lifecycleOwner: LifecycleOwner,
73+
chatIdFlow: Flow<String?>,
7174
onSearchSubmitted: (String) -> Unit,
7275
onCameraCaptureRequested: (ValueCallback<Array<Uri>>) -> Unit,
7376
onFilePickerRequested: (ValueCallback<Array<Uri>>, List<String>) -> Unit,
@@ -77,7 +80,7 @@ class RealContextualNativeInputManager @Inject constructor(
7780
this.widget = widget
7881

7982
applyCardShape(card)
80-
setupWidget(tabId, widget, onSearchSubmitted, onCameraCaptureRequested, onFilePickerRequested)
83+
setupWidget(tabId, widget, chatIdFlow, onSearchSubmitted, onCameraCaptureRequested, onFilePickerRequested)
8184
observeNativeInputSetting(lifecycleOwner)
8285
}
8386

@@ -113,11 +116,13 @@ class RealContextualNativeInputManager @Inject constructor(
113116
private fun setupWidget(
114117
tabId: String,
115118
widget: NativeInputModeWidget,
119+
chatIdFlow: Flow<String?>,
116120
onSearchSubmitted: (String) -> Unit,
117121
onCameraCaptureRequested: (ValueCallback<Array<Uri>>) -> Unit,
118122
onFilePickerRequested: (ValueCallback<Array<Uri>>, List<String>) -> Unit,
119123
) {
120124
widget.configureContextual(tabId)
125+
widget.bindChatIdSource(chatIdFlow)
121126
widget.hideMainButtons()
122127
widget.onStopTapped = ::sendStopEvent
123128
widget.bindAttachmentCallbacks(

‎duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualFragment.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,7 @@ class DuckChatContextualFragment :
433433
widget = binding.contextualNativeInputWidget,
434434
jsMessaging = contentScopeScripts,
435435
lifecycleOwner = viewLifecycleOwner,
436+
chatIdFlow = viewModel.chatId,
436437
onSearchSubmitted = { query ->
437438
viewModel.onContextualClose()
438439
startActivity(browserNav.openInNewTab(requireContext(), query))

‎duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModel.kt‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ class DuckChatContextualViewModel @Inject constructor(
8080
var updatedPageContext: String = ""
8181
var sheetTabId: String = ""
8282

83+
// Chat id currently shown in the contextual webview, derived from the URL query param.
84+
// Null when in INPUT mode (composing a new chat) or when the URL has no chatID yet.
85+
private val _chatId = MutableStateFlow<String?>(null)
86+
val chatId: StateFlow<String?> = _chatId.asStateFlow()
87+
8388
@VisibleForTesting
8489
internal var isPageContextRequested: Boolean = false
8590

@@ -172,6 +177,7 @@ class DuckChatContextualViewModel @Inject constructor(
172177
if (existingChatUrl == null) {
173178
val urlToLoad = duckChat.getDuckChatUrl("", false, sidebar = true)
174179
withContext(dispatchers.main()) {
180+
setSheetUrl(urlToLoad)
175181
commandChannel.trySend(Command.LoadUrl(urlToLoad))
176182
_viewState.update { state ->
177183
state.copy(
@@ -182,6 +188,7 @@ class DuckChatContextualViewModel @Inject constructor(
182188
}
183189
} else {
184190
withContext(dispatchers.main()) {
191+
setSheetUrl(existingChatUrl)
185192
_viewState.update { state ->
186193
state.copy(
187194
sheetMode = SheetMode.WEBVIEW,
@@ -233,6 +240,7 @@ class DuckChatContextualViewModel @Inject constructor(
233240
val hasChatHistory = hasChatId(existingChatUrl)
234241

235242
withContext(dispatchers.main()) {
243+
setSheetUrl(existingChatUrl)
236244
_viewState.update { current ->
237245
current.copy(
238246
sheetMode = SheetMode.WEBVIEW,
@@ -277,9 +285,9 @@ class DuckChatContextualViewModel @Inject constructor(
277285
if (url == null) {
278286
return
279287
} else {
280-
fullModeUrl = url
281288
val sheetMode = _viewState.value.sheetMode
282289
if (sheetMode == SheetMode.WEBVIEW) {
290+
setSheetUrl(url)
283291
val hasChatId = hasChatId(url)
284292

285293
viewModelScope.launch {
@@ -636,6 +644,7 @@ class DuckChatContextualViewModel @Inject constructor(
636644
contextualDataStore.clearTabChatUrl(currentTabId)
637645

638646
withContext(dispatchers.main()) {
647+
clearSheetUrl()
639648
_viewState.update {
640649
it.copy(
641650
sheetMode = SheetMode.INPUT,
@@ -655,10 +664,20 @@ class DuckChatContextualViewModel @Inject constructor(
655664
}
656665
}
657666

658-
private fun hasChatId(url: String?): Boolean {
659-
return url?.toUri()?.getQueryParameter(CHAT_ID_PARAM)
660-
.orEmpty()
661-
.isNotBlank()
667+
private fun hasChatId(url: String?): Boolean = !extractChatId(url).isNullOrBlank()
668+
669+
private fun extractChatId(url: String?): String? =
670+
url?.toUri()?.getQueryParameter(CHAT_ID_PARAM)?.takeIf { it.isNotBlank() }
671+
672+
// Owns the coupled (fullModeUrl, _chatId) invariant: _chatId is always extractChatId(fullModeUrl).
673+
private fun setSheetUrl(url: String) {
674+
fullModeUrl = url
675+
_chatId.value = extractChatId(url)
676+
}
677+
678+
private fun clearSheetUrl() {
679+
fullModeUrl = ""
680+
_chatId.value = null
662681
}
663682

664683
private suspend fun shouldReuseStoredChatUrl(tabId: String): Boolean {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,10 @@ class ReasoningModePickerViewModel @Inject constructor(
121121
fun onModeTapped(mode: ReasoningMode, surface: PickerSurface) {
122122
// Drop taps that race past the picker hiding (loading, mismatch, no accessible rows).
123123
if (!state.value.visible) return
124+
val modelState = modelManager.modelState.value
124125
val chat = currentChat.value
125-
val chatResolution = chat?.let { ReasoningResolver.forChat(it, modelManager.modelState.value) }
126-
val available = chatResolution?.available ?: modelManager.modelState.value.availableReasoningModes
126+
val chatResolution = chat?.let { ReasoningResolver.forChat(it, modelState) }
127+
val available = chatResolution?.available ?: modelState.availableReasoningModes
127128

128129
val match = available.firstOrNull { it.mode == mode }
129130
if (match == null) {
@@ -138,7 +139,7 @@ class ReasoningModePickerViewModel @Inject constructor(
138139
}
139140
return
140141
}
141-
val userTier = modelManager.modelState.value.userTier
142+
val userTier = modelState.userTier
142143
val requiredTier = match.access?.requiredTier ?: run {
143144
logcat { "Duck.ai reasoning picker: gated mode $mode has no public required tier, ignoring." }
144145
return

‎duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/DuckChatContextualViewModelTest.kt‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1201,6 +1201,67 @@ class DuckChatContextualViewModelTest {
12011201
assertNull(contextualDataStore.getTabChatUrl(tabId))
12021202
}
12031203

1204+
@Test
1205+
fun `chatId starts null`() {
1206+
assertNull(testee.chatId.value)
1207+
}
1208+
1209+
@Test
1210+
fun `when sheet opened with stored chat url then chatId set before page load`() = runTest {
1211+
val tabId = "tab-1"
1212+
val storedUrl = "https://duck.ai/chat?chatID=abc-123"
1213+
contextualDataStore.persistTabChatUrl(tabId, storedUrl)
1214+
1215+
testee.onSheetOpened(tabId)
1216+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1217+
1218+
assertEquals("abc-123", testee.chatId.value)
1219+
}
1220+
1221+
@Test
1222+
fun `onChatPageLoaded in WEBVIEW mode then chatId set from url`() = runTest {
1223+
val tabId = "tab-1"
1224+
val storedUrl = "https://duck.ai/chat?chatID=abc-123"
1225+
contextualDataStore.persistTabChatUrl(tabId, storedUrl)
1226+
testee.onSheetOpened(tabId)
1227+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1228+
1229+
val newUrl = "https://duck.ai/chat?chatID=xyz-789"
1230+
testee.onChatPageLoaded(newUrl)
1231+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1232+
1233+
assertEquals("xyz-789", testee.chatId.value)
1234+
}
1235+
1236+
@Test
1237+
fun `onChatPageLoaded in INPUT mode then chatId not set`() = runTest {
1238+
val tabId = "tab-1"
1239+
testee.onSheetOpened(tabId)
1240+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1241+
assertNull(testee.chatId.value)
1242+
1243+
val staleUrl = "https://duck.ai/chat?chatID=stale-123"
1244+
testee.onChatPageLoaded(staleUrl)
1245+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1246+
1247+
assertNull(testee.chatId.value)
1248+
}
1249+
1250+
@Test
1251+
fun `onNewChatRequested clears chatId`() = runTest {
1252+
val tabId = "tab-1"
1253+
val storedUrl = "https://duck.ai/chat?chatID=abc-123"
1254+
contextualDataStore.persistTabChatUrl(tabId, storedUrl)
1255+
testee.onSheetOpened(tabId)
1256+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1257+
assertEquals("abc-123", testee.chatId.value)
1258+
1259+
testee.onNewChatRequested()
1260+
coroutineRule.testDispatcher.scheduler.advanceUntilIdle()
1261+
1262+
assertNull(testee.chatId.value)
1263+
}
1264+
12041265
@Test
12051266
fun `onNewChatRequested clears stored url for current tab`() = runTest {
12061267
val tabId = "tab-1"

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,10 @@ class ReasoningModePickerViewModelTest {
263263
// Pathological: a "gated" entry whose access list still includes FREE → no upsell route.
264264
modelState.value = ModelState(
265265
userTier = UserTier.FREE,
266-
availableReasoningModes = listOf(gatedExtended(requires = listOf("free", "plus", "pro"))),
266+
availableReasoningModes = listOf(
267+
AvailableReasoningMode(ReasoningMode.FAST, ReasoningEffort.NONE),
268+
gatedExtended(requires = listOf("free", "plus", "pro")),
269+
),
267270
)
268271
runCurrent()
269272

@@ -280,7 +283,10 @@ class ReasoningModePickerViewModelTest {
280283
// Only non-public tiers in the access list → requiredTier is null → no upsell route.
281284
modelState.value = ModelState(
282285
userTier = UserTier.FREE,
283-
availableReasoningModes = listOf(gatedExtended(requires = listOf("internal"))),
286+
availableReasoningModes = listOf(
287+
AvailableReasoningMode(ReasoningMode.FAST, ReasoningEffort.NONE),
288+
gatedExtended(requires = listOf("internal")),
289+
),
284290
)
285291
runCurrent()
286292

@@ -295,7 +301,10 @@ class ReasoningModePickerViewModelTest {
295301
@Test
296302
fun whenTappedModeNotInAvailableListThenNoCommandEmittedAndManagerNotCalled() = runTest {
297303
modelState.value = ModelState(
298-
availableReasoningModes = listOf(AvailableReasoningMode(ReasoningMode.FAST, ReasoningEffort.NONE)),
304+
availableReasoningModes = listOf(
305+
AvailableReasoningMode(ReasoningMode.FAST, ReasoningEffort.NONE),
306+
AvailableReasoningMode(ReasoningMode.REASONING, ReasoningEffort.LOW),
307+
),
299308
)
300309
runCurrent()
301310

0 commit comments

Comments
 (0)