Skip to content

Per-chat model and reasoning restoration on existing Duck.ai chats.#8666

Merged
YoussefKeyrouz merged 2 commits into
developfrom
feature/youssef/update_model_and_reasoning_picker_per_chat
May 22, 2026
Merged

Per-chat model and reasoning restoration on existing Duck.ai chats.#8666
YoussefKeyrouz merged 2 commits into
developfrom
feature/youssef/update_model_and_reasoning_picker_per_chat

Conversation

@YoussefKeyrouz

@YoussefKeyrouz YoussefKeyrouz commented May 22, 2026

Copy link
Copy Markdown
Collaborator

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:

  • ReasoningModePickerViewModel observes nativeInputState.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.
  • NativeInputModeWidgetViewModel submission getters return the chat's stored model and a reasoning effort routed through ReasoningResolver.resolveMode
  • DuckAiModelManager adds a session-only chatScopedReasoningMode field with a setter; not persisted, cleared on chatId change.
  • Shared helper ReasoningResolver.forChat(chat, modelState) centralises the chat-aware resolution.

Steps to test this PR

Existing chat - restoration

  • Send a message in a new chat, pick a reasoning mode (e.g. Thinking)
  • Navigate away and reopen that chat from history
  • Picker displays the chat's stored mode; submitting sends the correct selection

New chat - global defaults

  • Open a new chat submit using any model
  • Open another new chat
  • Picker shows your Last used model

Mid-chat reasoning change (session-only)

  • In an existing chat, tap the picker and change the mode
  • Submit; submission uses the picked mode
  • Open a new chat - your global default is unchanged

Same-tab navigation between chats

  • In one tab, open chat A and pick a reasoning mode
  • Navigate to chat B in the same tab
  • Picker shows chat B's stored mode (not A's pick)

Tab switching

  • Open chat A in tab 1 and chat B in tab 2
  • Switch between tabs; each tab's picker shows its own chat's mode and submission uses that tab's chat

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 model and reasoning effort instead of always using the user’s global defaults.

This introduces a session-only chatScopedReasoningMode on DuckAiModelManager, adds ReasoningResolver.forChat(...) to resolve available modes/mode per chat model, and updates the input widget and reasoning picker to (a) load the active chat via DuckAiChatStore, (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 buffering chatId before configure().

Reviewed by Cursor Bugbot for commit caae8f5. Bugbot is set up for automated code reviews on this repo. Configure here.

YoussefKeyrouz commented May 22, 2026

Copy link
Copy Markdown
Collaborator Author

@YoussefKeyrouz YoussefKeyrouz force-pushed the feature/youssef/update_model_and_reasoning_picker_per_chat branch from 76382a3 to f4928f6 Compare May 22, 2026 14:42

@malmstein malmstein left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed

@YoussefKeyrouz YoussefKeyrouz force-pushed the feature/youssef/update_model_and_reasoning_picker_per_chat branch from fafa32f to b60ace4 Compare May 22, 2026 16:21
@YoussefKeyrouz YoussefKeyrouz force-pushed the feature/youssef/update_model_and_reasoning_picker_per_chat branch from b60ace4 to 2882db3 Compare May 22, 2026 19:26
@YoussefKeyrouz YoussefKeyrouz force-pushed the feature/youssef/update_model_and_reasoning_picker_per_chat branch from 2882db3 to 6f9a5f1 Compare May 22, 2026 19:54
Base automatically changed from feature/youssef/load_model_and_effort_per_chat to develop May 22, 2026 22:36
@YoussefKeyrouz YoussefKeyrouz force-pushed the feature/youssef/update_model_and_reasoning_picker_per_chat branch from 6f9a5f1 to caae8f5 Compare May 22, 2026 22:37

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

@YoussefKeyrouz YoussefKeyrouz merged commit f89c378 into develop May 22, 2026
16 checks passed
@YoussefKeyrouz YoussefKeyrouz deleted the feature/youssef/update_model_and_reasoning_picker_per_chat branch May 22, 2026 22:53
YoussefKeyrouz added a commit that referenced this pull request May 22, 2026
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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants