refactor: add pinia colada for better data loading and error handling #1099
Conversation
Add @pinia/colada dependency and useIpcQuery composable for type-safe IPC queries. Refactor MCP store to use Pinia Colada's query system with automatic caching, loading states, and error handling for config, tools, clients, resources, and prompts.
WalkthroughIntegrates Pinia Colada and IPC-backed query/mutation patterns across the renderer: adds type-safe composables (useIpcQuery/useIpcMutation), registers the Colada plugin, refactors multiple stores (mcp, prompts, sync, settings), adds a new searchEngine store, new events, docs/plans, and dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useIpcQuery
participant usePresenter
participant Presenter
participant Colada as Pinia_Colada
participant Cache
Component->>useIpcQuery: call(useIpcQuery(opts))
useIpcQuery->>usePresenter: getPresenter(name)
usePresenter->>Presenter: resolve
useIpcQuery->>Colada: useQuery(key, queryFn)
alt Cache hit
Colada->>Cache: read(key)
Cache-->>Colada: cached data
Colada-->>Component: return data
else Cache miss
Colada->>Presenter: invoke method(args)
Presenter-->>Colada: result
Colada->>Cache: store(key, result)
Colada-->>Component: return data
end
sequenceDiagram
participant Component
participant useIpcMutation
participant usePresenter
participant Presenter
participant Colada as Pinia_Colada
participant Cache
Component->>useIpcMutation: mutation.mutateAsync(vars)
useIpcMutation->>usePresenter: getPresenter(name)
usePresenter->>Presenter: resolve
useIpcMutation->>Presenter: call(vars)
Presenter-->>useIpcMutation: result
alt success
useIpcMutation->>Colada: invalidateQueries(keys)
Colada->>Cache: evict(keys)
useIpcMutation-->>Component: onSuccess(result)
else error
useIpcMutation-->>Component: onError(err)
end
useIpcMutation-->>Component: onSettled(result, err)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/stores/settings.ts (1)
334-361:initSettingsnow wires config/listeners comprehensively, butsetupProviderListeneris called twice
initSettings’ updated flow:
- Loads core settings and providers.
- Calls
searchEngineStore.initialize().- Refreshes all models and initializes the search assistant model.
- Sets up config‑related listeners (
setupContentProtectionListener,setupCopyWithCotEnabledListener,setupTraceDebugEnabledListener,setupFontSizeListener,setupSearchPreviewListener,setupNotificationsListener).- Calls
setupProviderListener().Later,
onMountedalso does:onMounted(async () => { await initSettings() await setupProviderListener() })Because
setupProviderListeneritself unconditionally registers multipleipcRenderer.onhandlers, calling it both insideinitSettingsand again inonMountedwill register each listener twice. That can cause duplicated updates, duplicated model refreshes, and extra work on every provider/config change.You likely only need to call
setupProviderListeneronce. A simple fix is to remove the explicit call insideinitSettingsand keep the one inonMounted, or vice versa (and drop it fromonMounted).- // 设置配置类事件监听器（确保实时同步状态） - setupContentProtectionListener() - setupCopyWithCotEnabledListener() - setupTraceDebugEnabledListener() - setupFontSizeListener() - setupSearchPreviewListener() - setupNotificationsListener() - - // 设置 provider 相关事件监听 - setupProviderListener() + // 设置配置类事件监听器（确保实时同步状态） + setupContentProtectionListener() + setupCopyWithCotEnabledListener() + setupTraceDebugEnabledListener() + setupFontSizeListener() + setupSearchPreviewListener() + setupNotificationsListener()(or alternatively drop the
await setupProviderListener()inonMounted).Also applies to: 150-155
🧹 Nitpick comments (11)
src/renderer/src/stores/floatingButton.ts (2)
11-11: Consider moving the guard into store state.While the module-level guard works for singleton stores, moving it into the store's reactive state would be clearer and handle edge cases like store disposal/recreation during development.
Apply this diff:
const configP = usePresenter('configPresenter') // 悬浮按钮是否启用的状态 const enabled = ref<boolean>(false) - let listenerRegistered = false + const listenerRegistered = ref<boolean>(false)And update the checks:
const setupFloatingButtonListener = () => { - if (listenerRegistered) { + if (listenerRegistered.value) { return } if (!window?.electron?.ipcRenderer) { return } - listenerRegistered = true + listenerRegistered.value = true
66-68: Consider explicit initialization overonMounted.Using
onMountedin a Pinia store is unconventional and ties initialization to component lifecycle, which could cause issues if the store is accessed from non-component contexts. Consider explicit initialization (e.g., callinginitializeState()frommain.tsafter app mount) for more predictable behavior..cursor/plans/store-colada-b9fc060f.plan.md (1)
1-192: Avoid hard-coding line numbers in long-lived plan docsThe plan references specific line numbers in files like
mcp.tsandsettings.ts. These are likely to drift as the code evolves, which can make the document misleading over time. Prefer pointing to functions/sections (e.g., “mcp.tscallToolmethod”) instead of raw line numbers so the plan stays accurate with fewer updates.src/renderer/src/composables/useIpcQuery.ts (1)
1-47: Type-safe query wrapper looks good; consider narrowing method keys to functionsThe abstraction around
useQueryand presenters looks clean and matches Pinia Colada’s API (key+query, plus selectedUseQueryOptionslikestaleTime/gcTime). (pinia-colada.esm.dev) Theargs?: MaybeRefOrGetter<Parameters<...>>design is also flexible for reactive arguments.One small type-level improvement you could consider (not urgent):
PresenterMethod<TName>currently allows any key ofIPresenter[TName], including non-function properties. You’re guarding at the type level viaPresenterMethodFn, but you can avoid accidental property picks by restricting to function-valued keys:-type PresenterName = keyof IPresenter -type PresenterMethod<TName extends PresenterName> = keyof IPresenter[TName] +type PresenterName = keyof IPresenter +type PresenterFunctionKeys<T> = { + [K in keyof T]: T[K] extends (...args: any[]) => any ? K : never +}[keyof T] +type PresenterMethod<TName extends PresenterName> = PresenterFunctionKeys<IPresenter[TName]>You could then reuse these aliases in
useIpcMutation.tsto avoid duplicating the presenter method typing logic.Please confirm this still lines up with the current
IPresenterdefinition and doesn’t exclude any presenter members you intentionally want to expose here.docs/mcp-store-colada-integration.md (1)
35-35: Add a language hint to the fenced diagram block.markdownlint (MD040) flags this code fence because it lacks a language tag. Please annotate the block as plain text (for example,
text) to satisfy the lint rules.-``` +```textsrc/renderer/src/stores/sync.ts (3)
22-44: Backups query wiring and refresh behavior look consistent with Colada usage
backupQueryKey,backupsQuery,backups(sorted bycreatedAt), andrefreshBackupsare wired coherently: a stable key, IPC-backed query, derived sorted list, and an explicit refetch helper with error logging. Given that you already invalidate this key in the mutations below, this should keep the list fresh after local backup/import operations.One behavioral gap to consider: backups created from other windows or background actions will only show up after TTL expiry or a manual
refreshBackups()call. If you want immediate UI updates when those events fire, consider also callingrefreshBackups()inside theSYNC_EVENTS.BACKUP_COMPLETEDandSYNC_EVENTS.IMPORT_COMPLETEDlisteners ininitialize.
45-101: Backup/import mutation flows are guarded well; minor subtlety aroundisBackingUp/eventsThe
startBackupandimportDatahelpers add good guardrails:
- Short‑circuit when sync is disabled, already backing up/importing, or
backupFileis empty.- Local
isBackingUp/isImportingflags withtry/finallyreset.- Cache invalidation via
invalidateQueries: () => [backupQueryKey()], plus an extrarefreshBackups()on success and in the importfinallyblock.The only subtlety is that
isBackingUpis now controlled both by this function and by IPC events (SYNC_EVENTS.BACKUP_STARTED/BACKUP_COMPLETED). If the presenter returns quickly while the backup still runs,startBackup’sfinallymay flipisBackingUpback tofalsebefore the completion event arrives. That likely only causes a brief UI mismatch but is worth being aware of; if it becomes an issue, you could rely solely on IPC events for the flag and letstartBackupavoid toggling it directly.
172-186: Sync settings listener correctly refreshes on folder changes; small simplification possible
setupSyncSettingsListenerkeepssyncEnabledandsyncFolderPathin sync withCONFIG_EVENTS.SYNC_SETTINGS_CHANGED, and triggersrefreshBackups()only when the folder actually changes, which avoids redundant reloads.The
else if (typeof payload.folderPath === 'string')branch effectively reassigns the same value without refreshing. This is harmless but can be simplified by computing ahasFolderChangedboolean once and then updating/refreshing accordingly, if you want to trim conditionals.You might also consider whether changes to
enabledalone should trigger any cleanup (e.g., clearingbackupsorimportResult) when sync is turned off, depending on UI expectations.src/renderer/src/stores/mcp.ts (2)
62-85: Config query + watchers keep MCP config in sync; slight redundancy but acceptableThe
configQuery(['mcp', 'config']) aggregates servers, defaults, and enabled state in one shot and normalizes nullish values, which is good.syncConfigFromQueryand the watcher onconfigQuery.data.value(with{ immediate: true }) ensureconfig.valueis updated whenever the query data changes.
loadConfigalso callsrunQuery(configQuery, options)and then callssyncConfigFromQuery(state.data)again on success. That means for the initial load you effectively sync config twice to the same value; this isn’t harmful, just a bit redundant.If you want to tighten things up, you could rely solely on the watcher for syncing (
loadConfigwould justawait runQuery(configQuery, options)and thenupdateAllServerStatuses()whenconfig.value.readyflips), but the current implementation is correct.Also applies to: 203-215, 238-267
305-363: Mutation + invalidation design looks correct; small potential to rely more on query invalidationThe new mutations for server/config operations (
addServer*,updateServer*,removeServer*,addDefaultServer*,resetToDefaultServers*,setMcpEnabledMutation) consistently:
- Target
mcpPresentermethods.- Invalidate the appropriate query keys (config/tools/clients/resources as needed).
In the higher‑level methods (
addServer,updateServer,removeServer,toggleDefaultServer,resetToDefaultServers), you’re also callingrunQuery(configQuery, { force: true })after successful mutations. That ensuresconfigis eagerly resynced, but it’s somewhat redundant with the invalidation, which will already cause queries to refetch on next use.Not a correctness issue, but if you want to reduce duplicate network calls you could consider relying on invalidation alone and only forcing a
configQueryrefresh in flows that must synchronously see the updated config.Also applies to: 430-506
src/renderer/src/stores/settings.ts (1)
1557-1571: Extended cleanup covers new listeners; consider whether it should own SEARCH_ENGINES_UPDATED cleanupThe
cleanupfunction now removes listeners for:
CONTENT_PROTECTION_CHANGEDCOPY_WITH_COT_CHANGEDTRACE_DEBUG_CHANGEDFONT_SIZE_CHANGEDSEARCH_PREVIEW_CHANGEDNOTIFICATIONS_CHANGEDwhich matches the new
setup*Listenerfunctions and helps avoid leaking IPC handlers.One thing to re‑evaluate:
cleanupstill callsremoveAllListeners(CONFIG_EVENTS.SEARCH_ENGINES_UPDATED), but the settings store no longer sets up that listener itself—it’s delegated tosearchEngineStore.setupSearchEnginesListener(). If other parts of the app useuseSearchEngineStore, this cleanup might unexpectedly remove their listeners. Ifcleanupis only called during global teardown this is harmless, but if it can be invoked earlier you may want to move SEARCH_ENGINES_UPDATED cleanup into the search engine store instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.cursor/plans/store-colada-b9fc060f.plan.md(1 hunks)docs/mcp-store-colada-integration.md(1 hunks)package.json(2 hunks)src/main/events.ts(1 hunks)src/main/presenter/configPresenter/index.ts(3 hunks)src/renderer/src/components/McpToolsList.vue(1 hunks)src/renderer/src/composables/useIpcMutation.ts(1 hunks)src/renderer/src/composables/useIpcQuery.ts(1 hunks)src/renderer/src/events.ts(1 hunks)src/renderer/src/main.ts(2 hunks)src/renderer/src/stores/floatingButton.ts(2 hunks)src/renderer/src/stores/mcp.ts(15 hunks)src/renderer/src/stores/prompts.ts(1 hunks)src/renderer/src/stores/searchEngineStore.ts(1 hunks)src/renderer/src/stores/settings.ts(9 hunks)src/renderer/src/stores/sync.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-12T00:58:06.513Z
Learnt from: zerob13
Repo: ThinkInAIXYZ/deepchat PR: 977
File: package.json:137-137
Timestamp: 2025-10-12T00:58:06.513Z
Learning: In this Electron project, renderer process dependencies (used only in src/renderer/) should be placed in devDependencies because they are bundled by electron-vite during the build process. Only main process dependencies (used in src/main/) need to be in the dependencies section.
Applied to files:
package.json
🧬 Code graph analysis (9)
src/renderer/src/composables/useIpcMutation.ts (3)
src/shared/types/presenters/legacy.presenters.d.ts (1)
IPresenter(353-373)src/main/presenter/index.ts (1)
presenter(223-223)src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)
src/renderer/src/stores/floatingButton.ts (2)
src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)src/renderer/src/events.ts (1)
FLOATING_BUTTON_EVENTS(152-157)
src/renderer/src/stores/prompts.ts (3)
src/renderer/src/composables/useIpcQuery.ts (1)
useIpcQuery(31-47)src/shared/types/presenters/legacy.presenters.d.ts (1)
Prompt(58-74)src/renderer/src/composables/useIpcMutation.ts (1)
useIpcMutation(40-89)
src/renderer/src/stores/searchEngineStore.ts (5)
src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)src/renderer/src/composables/useIpcQuery.ts (1)
useIpcQuery(31-47)src/renderer/src/composables/useIpcMutation.ts (1)
useIpcMutation(40-89)src/main/presenter/threadPresenter/index.ts (2)
setSearchEngine(213-220)testSearchEngine(204-206)src/renderer/src/events.ts (1)
CONFIG_EVENTS(11-33)
src/main/presenter/configPresenter/index.ts (3)
src/main/eventbus.ts (1)
eventBus(151-151)src/main/events.ts (2)
CONFIG_EVENTS(12-43)FLOATING_BUTTON_EVENTS(198-207)src/renderer/src/events.ts (2)
CONFIG_EVENTS(11-33)FLOATING_BUTTON_EVENTS(152-157)
src/renderer/src/stores/sync.ts (5)
src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)src/renderer/src/composables/useIpcQuery.ts (1)
useIpcQuery(31-47)src/shared/types/presenters/legacy.presenters.d.ts (1)
SyncBackupInfo(1379-1383)src/renderer/src/composables/useIpcMutation.ts (1)
useIpcMutation(40-89)src/renderer/src/events.ts (1)
CONFIG_EVENTS(11-33)
src/renderer/src/stores/mcp.ts (6)
src/shared/types/presenters/legacy.presenters.d.ts (5)
MCPConfig(1144-1149)ResourceListEntry(121-128)PromptListEntry(84-97)Prompt(58-74)MCPServerConfig(1128-1142)src/renderer/src/composables/useIpcQuery.ts (1)
useIpcQuery(31-47)src/main/presenter/mcpPresenter/mcpClient.ts (1)
callTool(1160-1200)src/renderer/src/composables/useIpcMutation.ts (1)
useIpcMutation(40-89)src/main/presenter/mcpPresenter/index.ts (1)
callTool(543-587)src/main/presenter/mcpPresenter/toolManager.ts (1)
callTool(304-453)
src/renderer/src/composables/useIpcQuery.ts (2)
src/shared/types/presenters/legacy.presenters.d.ts (1)
IPresenter(353-373)src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)
src/renderer/src/stores/settings.ts (2)
src/renderer/src/stores/searchEngineStore.ts (1)
useSearchEngineStore(10-148)src/renderer/src/events.ts (1)
CONFIG_EVENTS(11-33)
🪛 LanguageTool
docs/mcp-store-colada-integration.md
[uncategorized] ~1094-~1094: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：便捷"地"访问
Context: ... 或 useQuery 定义查询 4. 添加计算属性：为组件提供便捷的访问接口 5. 添加缓存失效：在相关的 Mutation 中配置缓存失效...
(wb4)
[uncategorized] ~1257-~1257: 动词的修饰一般为‘形容词（副词）+地+动词’。您的意思是否是：友好"地"调用
Context: ...s` 中列出需要失效的查询 5. 包装 Store 方法：为组件提供友好的调用接口 6. 错误处理：添加适当的错误处理和用户反馈 #### 代码...
(wb4)
🪛 markdownlint-cli2 (0.18.1)
docs/mcp-store-colada-integration.md
35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (14)
src/renderer/src/stores/floatingButton.ts (2)
4-4: LGTM!The import is necessary for the new IPC event listener and is correctly used.
40-40: LGTM!The listener setup is correctly placed after state initialization.
package.json (2)
107-107: @pinia/colada correctly scoped as a renderer devDependencyUsing
@pinia/coladaonly from renderer stores/composables and registering it insrc/renderer/src/main.tsmakesdevDependenciesthe right place in this Electron setup, since renderer code is bundled by electron-vite. This matches the existing convention of keeping renderer-only libraries out ofdependencies. Based on learningsPlease double-check there is no main-process (src/main/) usage of Pinia Colada so we don’t accidentally require it at runtime outside the bundle.
169-169: Confirm vue-renderer-markdown 0.0.62-beta.1 compatibilityThe minor/beta bump for
vue-renderer-markdownlooks reasonable, but renderer Markdown rendering is a user-facing path, so it’s worth a quick regression check (e.g., headings, lists, code blocks) to ensure there are no breaking changes or styling regressions.src/renderer/src/main.ts (1)
6-35: Pinia Colada plugin wiring and global query options look correctRegistering
PiniaColadaaftercreatePinia()and usingqueryOptionsfor global defaults matches the library’s recommended setup. (github.com) The chosenstaleTime(30s) andgcTime(5min) also align with the plan’s intent to keep IPC-backed renderer data warm while avoiding overly long retention.Just make sure these global values work well for all current queries (MCP, prompts, sync, search engines); if any resource needs different staleness, you can still override
staleTime/gcTimeper-query viauseIpcQuery.src/renderer/src/stores/mcp.ts (6)
55-61:runQueryhelper integrates well with Colada; be mindful of when you wantrefetchvsrefreshThe
QueryExecuteOptions+runQueryhelper cleanly abstracts choosing betweenqueryReturn.refetch(forforce: true) andqueryReturn.refresh(default). Your uses inloadConfig,loadClients,loadTools,loadPrompts, andloadResourcesare consistent and checkstate.status === 'success'when they need the returned data.Two things to verify:
- That the
UseQueryReturnfrom@pinia/coladaindeed exposes bothrefreshandrefetchwith the same return type (QueryState<T>), matching your usage.- That callers are clear on the semantics:
force: truewill bypass staleness checks viarefetch, while the defaultrefreshrespects staleness.Functionally this looks solid and gives you a straightforward way to control cache behavior per call site.
Also applies to: 366-379, 542-589
87-110: IPC-backed tools/clients/resources/prompts queries and derived computed state look correct
toolsQuery,clientsQuery, andresourcesQueryall useuseIpcQuerywith:
- Stable keys under the
'mcp'namespace.enabled: () => config.value.ready && config.value.mcpEnabledso they only run once config has loaded and MCP is enabled.promptsQuerymerges custom prompts from config with MCP prompts when enabled, and falls back to only custom prompts otherwise.- Computed
tools,clients,resources, andpromptscorrectly gate onmcpEnabledand default to empty arrays when data is absent.toolsLoading,toolsError, andtoolsErrorMessagewraptoolsQuery’s reactive state into simple consumable flags/messages.This wiring is consistent and should integrate well with the mutation invalidation patterns defined later.
Also applies to: 154-163, 188-202
111-153: Prompt loading separation between MCP and config presenters is clear and resilientSplitting prompt loading into
loadMcpPrompts(viamcpPresenter) andloadCustomPrompts(viaconfigPresenterandPrompt→PromptListEntrymapping) gives a clear source separation and allows custom prompts to work even when MCP is disabled.The mapping preserves fields (
name,description,parameters→arguments,files, and a syntheticclientidentity), and both loaders log and return[]on failure, which is a reasonable fallback.This is a solid improvement for prompt robustness around MCP availability.
164-187: Tool call mutation and helper are coherent; ensure event-based and mutation-based flows stay alignedThe call stack for tool invocation is nicely factored:
CallToolRequest/Result/MutationVarsreuse the actual presenter types, keeping everything type-aligned.callToolMutationusesuseIpcMutationwith:
onSuccessupdatingtoolResults[toolName]fromresult.content.onErrorlogging a localized error and populatingtoolResults[toolName]with a user-friendly message.callToolbuilds a request fromtoolInputs, appliessearch_filesdefaults, and callsmutateAsync([request])while togglingtoolLoadingStates[toolName]in atry/finally.Assuming
mcpPresenter.callToolreturns exactly the{ content: string; rawData: MCPToolResponse }(or similar) that yourCallToolResultalias expects, this path should be reliable. Just keep the earlierMCP_EVENTS.TOOL_CALL_RESULTevent payload in sync so that tool results updated from background events match the same structure.Also applies to: 600-632
216-236: Tool snapshot initialization and config-driven cleanup behave as expected
applyToolsSnapshotinitializestoolInputslazily per tool, with special defaults for thesearch_filestool. The watcher ontoolsQuery.data.value:
- Bails out when MCP is disabled.
- Applies snapshots immediately and on subsequent updates when tool definitions change.
The watcher on
config.value.mcpEnabledresets bothtoolInputsandtoolResultsto{}when MCP is disabled, which is a good cleanup step to avoid stale UI.All of this is consistent with the rest of the state management in the store.
Also applies to: 244-267
542-555: Client and resource loaders chain dependent queries in a sensible order
loadClients’ early return when MCP is disabled prevents unnecessary work. When it does run, you:
- Refresh
clientsQuery(respectingforce), then- On success, call
loadPrompts(options)andloadResources(options).This ensures prompts/resources are refreshed only once clients have been (re)loaded. Similarly,
loadResourcesbails out when MCP is disabled and otherwise just refreshesresourcesQuery. Error handling is localized and logs appropriately.This sequencing looks correct and is a nice improvement over ad‑hoc presenter calls.
Also applies to: 579-589
src/renderer/src/stores/settings.ts (3)
1-2: Search engine store integration centralizes search state cleanlySwitching to
useSearchEngineStoreplusstoreToRefsand exposingsearchEngines/activeSearchEnginefrom there is a solid refactor. It removes duplicated search engine state fromsettingsand delegates:
- Initialization (
searchEngineStore.initialize()),- Active engine selection,
- CRUD/test behaviors
to the dedicated store, which matches the PR’s goal of centralizing data loading.
The return aliases (
setSearchEngine,refreshSearchEngines,testSearchEngine,setupSearchEnginesListener) at the bottom are a nice way to preserve the existing public surface while routing through the new implementation.Also applies to: 18-19, 24-32
1680-1697: New search preview & notifications listeners align with config events
setupSearchPreviewListenerandsetupNotificationsListenermirror the existing pattern for other config flags:
- They listen to
CONFIG_EVENTS.SEARCH_PREVIEW_CHANGEDandCONFIG_EVENTS.NOTIFICATIONS_CHANGED.- They update the local
searchPreviewEnabledandnotificationsEnabledrefs directly.This keeps UI state in sync with external changes and matches how other toggles (content protection, copy-with-CoT, trace debug, font size) are handled. Combined with the additions to
cleanup, this is a straightforward and correct extension.
1896-1951: Public API aliases to search engine store maintain backward compatibilityAt the return block, the additions:
searchEngines,activeSearchEngine,setSearchEngine: searchEngineStore.setSearchEngine,testSearchEngine: searchEngineStore.testSearchEngine,refreshSearchEngines: searchEngineStore.refreshSearchEngines,setupSearchEnginesListener: searchEngineStore.setupSearchEnginesListener,ensure existing callers of
useSettingsStorecontinue to work without being aware of the newuseSearchEngineStore. This is an appropriate compatibility layer for a refactor of this scope.No functional issues here; the aliasing looks correct.
Summary by CodeRabbit
New Features
Chores
Refactor