Add per-tab NativeInputStateProvider/Publisher#8549
Merged
Conversation
Introduces NativeInputStateProvider (read) and NativeInputStatePublisher (write) in duck-chat-api, backed by a per-tab MutableStateFlow map in RealNativeInputStateStore. Moves NativeInputState (and its enums) from duckchat-impl to duck-chat-api so observers can depend on the API module. TabDataRepository now calls publisher.clearTab(tabId) at the three delete sites so per-tab state does not leak after a tab is closed. Pure plumbing: no widget or plugin reads or writes through these interfaces yet. Follow-up PRs migrate the widget to publish and plugins to observe, then remove NativeInputHost.getInputState(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ 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 c54c5ae. Configure here.
3 tasks
Adds clearAll() to NativeInputStatePublisher and wires the publisher into the three tab-removal paths that were missed in the initial wiring: replaceTabWithNewTab and purgeDeletableTabs now call clearTab, and deleteAll calls clearAll. Without this, per-tab native input state outlives the tabs it belongs to and leaks memory through RealNativeInputStateStore's ConcurrentHashMap, while every other per-tab store (siteData, contextual chat, visited sites) is cleared. Flagged by Cursor Bugbot on the original PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| } | ||
|
|
||
| override fun clearTab(tabId: String) { | ||
| flows.remove(tabId) |
Collaborator
There was a problem hiding this comment.
I think anything that was listening to this flow will keep the last value emitted even after removing it. So it will become stale. it’s not a problem now because nothing is listening to it but good to keep in mind for follow up PRs.
YoussefKeyrouz
approved these changes
May 14, 2026
Collaborator
YoussefKeyrouz
left a comment
YoussefKeyrouz
left a comment
There was a problem hiding this comment.
LGTM as a first scaffolding.
malmstein
added a commit
that referenced
this pull request
May 15, 2026
Task/Issue URL: https://app.asana.com/1/137249556945/project/1174433894299346/task/1214788735007211?focus=true Stacked on top of #8549. ### Description PR 2 of the native input state refactor — see the parent task for the full plan. The native input widget is now the single publisher of `NativeInputState` and reads its own state from the per-tab store (`NativeInputStateProvider`) instead of keeping it as a widget-local field. Plugins still won't see this change until PR 3. Concretely: - `NativeInputModeWidgetViewModel` injects `NativeInputStatePublisher`. A new `init { … }` block collects `combine(state, activeTabId)` and pushes each value through `publisher.publish(tabId, state.copy(tabId = tabId))` whenever a tabId is set. - `NativeInputModeWidget` injects `NativeInputStateProvider`. `observeNativeInputState()` now subscribes to `provider.stateForTab(tabId)` instead of `viewModel.state`. `getInputState()` reads `provider.stateForTab(activeTabId).value`. - `configure()` and `configureContextual()` take a `tabId: String` parameter and start the observation once the tabId is known. - `tabId` is threaded: - `BrowserTabFragment.tabId` → `NativeInputManager.showNativeInput(tabId, …)` → `attachWidget(widgetView, isBottom, tabId)` → `widget.configure(tabId, …)`. - `DuckChatContextualFragment` reads its `KEY_DUCK_AI_CONTEXTUAL_TAB_ID` argument once and passes it to `ContextualNativeInputManager.init(tabId, …)` → `widget.configureContextual(tabId)`. - `viewModel.state` stays as a `SharedFlow<NativeInputState>` so existing ViewModel-level tests can keep asserting on its values; the widget no longer consumes it. What this PR intentionally does **not** do: - `NativeInputState.tabId` stays nullable. Tightening to non-null will follow once the publish-on-configure invariant is verified in practice (and we can drop the `zero("__init__")`-style placeholder). - `NativeInputHost.getInputState()` is still exposed — its implementation now reads from the provider, but the surface remains until plugins are migrated in PR 3 and it's removed in PR 4. - Plugins still receive the widget as their `NativeInputHost`; nothing about their behaviour changes here. ### Steps to test this PR - [x] CI green: `:duckchat-impl:testDebugUnitTest`, `:app:testPlayDebugUnitTest`, `spotlessCheck`. - [x] Existing `NativeInputModeWidgetViewModelTest` cases still pass after the `configure`/`configureContextual` signature changes and the added publisher mock. - [x] Smoke-test on device: - Open Duck.ai input from the NTP, type, switch to the chat tab, submit — behaviour matches develop. - Open the omnibar on a browser tab, switch between Search and Duck.ai modes — toggle/position/context behave as before. - Open Duck.ai contextual chat from a browser tab — keyboard, attachments, and submit all still work. - Background a tab and reopen it — input state for that tab is restored correctly. ### UI changes None. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes how the native input widget publishes/consumes state by keying everything on `tabId`, which could cause cross-tab state leaks or missing updates if tab IDs aren’t consistently threaded through all entry points. > > **Overview** > The native input widget now **publishes `NativeInputState` per tab** and **reads state from `NativeInputStateProvider`** instead of relying on widget-local/ViewModel flow state. > > This threads `tabId` through `BrowserTabFragment` → `NativeInputManager.showNativeInput`/`attachWidget` → `NativeInputModeWidget.configure`, and through contextual chat via `DuckChatContextualFragment`/`ContextualNativeInputManager.init` → `configureContextual`. The ViewModel now injects `NativeInputStatePublisher` to push updates for the active tab, and tests are updated for the new `tabId`-required `configure` APIs. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9dc03e8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1174433894299346/task/1214788735007211?focus=true
Description
First PR in a series moving native input state from
NativeInputHost.getInputState()(pull-based, single in-memory snapshot in the widget) to a per-tab, push-based store that any consumer can observe.This PR is pure plumbing — no behavior change. No widget or plugin writes or reads through the new interfaces yet.
What lands here:
duck-chat-apigets two new interfaces undercom.duckduckgo.duckchat.api.nativeinput:NativeInputStateProvider— read side, exposesstateForTab(tabId: String): StateFlow<NativeInputState>.NativeInputStatePublisher— write side, withpublish(tabId, state),update(tabId, transform), andclearTab(tabId).Only the native input widget should depend on the publisher; everything else observes via the provider.
NativeInputState(and its nestedInputMode/InputContext/InputPosition/ToggleSelection) moves fromduckchat-impltoduck-chat-api. A newNativeInputState.zero(tabId)factory documents the placeholder default; a TODO marks that the widget will overwrite it once wired to publish on configure (next PR).RealNativeInputStateStoreinduckchat-implbinds both interfaces atAppScope, backed by aConcurrentHashMap<String, MutableStateFlow<NativeInputState>>.TabDataRepositoryinjects the publisher and callsclearTab(tabId)at the three delete sites (delete,deleteTabs,deleteTabAndSelectSource) so per-tab state does not leak after a tab closes.Follow-up PRs:
NativeInputStatePublisherand read its own state from the provider; tightentabIdto non-null.NativeInputPluginViewModels to observestateForTab(tabId); route any plugin contributions through new typedNativeInputHostmethods that the widget translates into publisher updates.NativeInputHost.getInputState()andNativeInputPlugin.getPromptContribution().Steps to test this PR
Plumbing only — no user-visible change.
:duckchat-api:compileDebugKotlin,:duckchat-impl:testDebugUnitTest,:app:testPlayDebugUnitTestRealNativeInputStateStoreTestpasses (9 cases covering per-tab isolation,updateatomicity,clearTab, zero default).TabDataRepositoryTeststill passes after constructor signature update.develop.UI changes
None.
Note
Medium Risk
Introduces new state-store plumbing and updates tab-deletion paths to clear per-tab native input state; risk is mainly around DI wiring and ensuring state is properly evicted on tab lifecycle events.
Overview
Adds a new per-tab, push-based native input state API (
NativeInputStateProvider/NativeInputStatePublisher) backed by anAppScopeRealNativeInputStateStorethat maintains aStateFlowper tab.Moves
NativeInputStateintoduckchat-api, extends it with an optionaltabIdand azero()placeholder factory, and updates existing widget/plugin/viewmodel/test imports to reference the new API package.Updates
TabDataRepositorytab cleanup (delete,deleteTabs,replaceTabWithNewTab,purgeDeletableTabs,deleteTabAndSelectSource,deleteAll) to evict native input state via the publisher, with corresponding test assertions and new unit coverage for the store.Reviewed by Cursor Bugbot for commit a038cf8. Bugbot is set up for automated code reviews on this repo. Configure here.