Per-chat model and reasoning restoration on existing Duck.ai chats.#8666
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
76382a3 to
f4928f6
Compare
malmstein
left a comment
There was a problem hiding this comment.
did not install or smoke-test the APK.
the chatId race I flagged on #8664 is fixed exactly as suggested — _chatId + combine is gone, setActiveChatId now uses the synchronous activeTabId.value ?: return; publisher.update(tabId) { ... } pattern mirroring setSelectedTool. nice.
the test coverage on this PR is genuinely impressive — the CompletableDeferred race tests for "lookup in flight" and "chatId flips mid-load" are exactly the kind of coverage I was nervous about not having for the prior PR. validChat()'s "currentChat.value only counts if it matches the published chatId" guard is the right shape. likewise the picker's hide-on-mismatch + popup-dismiss-on-hide combo prevents stale taps from landing in the wrong chat scope.
main question is the model fallback asymmetry below — it's an edge case (chat's model removed/renamed by the backend) but worth confirming the semantics. small future note: currentChat is loaded independently in both the widget VM and the picker VM via separate getChatById calls. fine for now but worth keeping an eye on as more consumers want chat-derived state.
|
|
||
| fun getSelectedModelId(): String? { | ||
| // Existing chat: send chat's stored model. | ||
| validChat()?.model?.let { return it } |
There was a problem hiding this comment.
when the chat's model is no longer in modelState.models (server stopped offering it, slug renamed, etc.), getSelectedModelId here still returns validChat().model unconditionally — but getResolvedReasoningEffort falls back to the global effort via forChat returning null. submission then carries (stale chat model, global effort), which is a weird pairing the backend wasn't designed for. the picker already hides itself for this case (chatResolution == null in resolveState), so the user has no UI signal that anything's off either. shouldn't getSelectedModelId fall back symmetrically — ReasoningResolver.forChat(chat, modelState) == null → use global for both? happy to be wrong if the backend tolerates unknown model ids gracefully, just want to make sure it's deliberate.
fafa32f to
b60ace4
Compare
b60ace4 to
2882db3
Compare
2882db3 to
6f9a5f1
Compare
6f9a5f1 to
caae8f5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit caae8f5. Configure here.
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 -->


Task/Issue URL: https://app.asana.com/1/137249556945/project/1212810093780571/task/1215057967719589?focus=true
Description
Tech design: https://app.asana.com/1/137249556945/project/1212810093780571/task/1214935104654912?focus=true
PR 3/3
When opening an existing Duck.ai chat, the input widget now restores the chat's stored model and reasoning effort instead of using the user's global defaults. Submissions in an existing chat carry the chat's values; the global "last used" model/reasoning remain untouched.
Implementation notes:
ReasoningModePickerViewModelobservesnativeInputState.chatId, looks up the chat, and derives picker rows from the chat's stored model; displayed mode prefers the session pick over the chat's stored mode.NativeInputModeWidgetViewModelsubmission getters return the chat's stored model and a reasoning effort routed throughReasoningResolver.resolveModeDuckAiModelManageradds a session-onlychatScopedReasoningModefield with a setter; not persisted, cleared on chatId change.ReasoningResolver.forChat(chat, modelState)centralises the chat-aware resolution.Steps to test this PR
Existing chat - restoration
Thinking)New chat - global defaults
Mid-chat reasoning change (session-only)
Same-tab navigation between chats
Tab switching
Note
Medium Risk
Medium risk because it changes how model/reasoning are resolved and submitted for existing chats, adding new session-scoped state and async chat lookups that could affect selection correctness across tab/chat switches.
Overview
Existing Duck.ai chats now restore and submit their stored
modeland reasoning effort instead of always using the user’s global defaults.This introduces a session-only
chatScopedReasoningModeonDuckAiModelManager, addsReasoningResolver.forChat(...)to resolve available modes/mode per chat model, and updates the input widget and reasoning picker to (a) load the active chat viaDuckAiChatStore, (b) hide/drop interactions during in-flight chat switches or when a chat’s model is no longer available, and (c) clear chat-scoped overrides on chat changes so global preferences are not persisted/overwritten. Tests are expanded to cover chat switching, missing models, gated modes, and bufferingchatIdbeforeconfigure().Reviewed by Cursor Bugbot for commit caae8f5. Bugbot is set up for automated code reviews on this repo. Configure here.