src/main/presenter/configPresenter/providerModelHelper.ts (2)

15-26: Align ModelConfigResolver typing with actual usage and sync expectation

ModelConfigResolver is typed to always return a ModelConfig, but getProviderModels treats config as possibly absent:

const config = this.getModelConfig(model.id, providerId)
if (config) {
  // ...
} else {
  // ...
}

and the helper assumes a synchronous resolver (it immediately reads config.maxTokens, etc.).

To avoid subtle bugs and type gaps:

• Either change the type to type ModelConfigResolver = (modelId: string, providerId?: string) => ModelConfig | undefined, or
• If a config must always exist, drop the if (config) branch and enforce that invariant at the resolver.

Also ensure that the concrete resolver you pass here is synchronous and does not return a Promise; otherwise this code will start reading properties off a Promise at runtime.

Also applies to: 57-63

---

57-82: Consider avoiding in-place mutation of ElectronStore model objects

getProviderModels and getCustomModels mutate the MODEL_META objects returned from store.get(...) before returning them. This works, but it ties callers to ElectronStore’s internal object identity.

For clearer separation and safer future changes, consider returning new objects instead:

return models.map((model) => {
  const config = this.getModelConfig(model.id, providerId)
  const base = { ...model }
  // apply overrides to `base` instead of mutating `model`
  return base
})

Similar treatment can be applied in getCustomModels.

Also applies to: 89-99

src/renderer/settings/components/OllamaProviderSettingsDetail.vue (3)

288-305: Verify that Ollama model names align with RENDERER_MODEL_META.id

providerModelMetas is keyed by meta.id, but you later look up metadata using the Ollama model’s name:

const metaMap = new Map(
  providerModelMetas.value.map((meta) => [meta.id, meta])
)

const meta = metaMap.get(model.name)

This assumes that, for the Ollama provider, RENDERER_MODEL_META.id equals the Ollama model name. If IDs are prefixed (e.g. ollama/gemma3:4b) while model.name is just gemma3:4b, meta will always be undefined and you’ll silently lose persisted flags like enabled, vision, etc.

Please double-check that:

• modelStore.allProviderModels exposes IDs that match localModels’ name values for Ollama, or
• Otherwise, adjust the keying to use a shared canonical ID (e.g. meta.ollamaModel?.name or a normalized key function).

Also applies to: 315-325, 759-793

---

135-152: Ensure model status updates use the same ID convention as model metadata

Here you pass model.name into the status API:

<ModelConfigItem
  :model-id="model.meta?.id ?? model.name"
  ...
  @enabled-change="handleModelEnabledChange(model.name, $event)"
/>

const handleModelEnabledChange = async (modelName: string, enabled: boolean) => {
  await modelStore.updateModelStatus(props.provider.id, modelName, enabled)
}

If modelStore.updateModelStatus and the underlying status helper use the canonical model ID (often meta.id), but you pass only the display model.name, status entries may not align with the metadata used elsewhere (especially if IDs are prefixed or normalized differently).

It would be safer to:

• Pass model.meta?.id ?? model.name into handleModelEnabledChange, and
• Use that same identifier consistently for status/config operations.

Please verify that the expected identifier for updateModelStatus matches what you pass here.

Also applies to: 767-793, 870-873

---

314-316: Update outdated comment; provider/ollama store usage looks good

The logic now correctly routes:

• Model refresh and locality checks through ollamaStore,
• Status updates through modelStore, and
• API updates, key validation, and provider deletion through providerStore.

The inline comment “使用 settings store” above refreshModels is now misleading, since the settings store is no longer used—consider updating or removing it. Otherwise, the refactor here looks coherent.

Also applies to: 850-853, 903-904, 908-909, 916-917, 920-923, 945-948

src/main/presenter/configPresenter/modelStatusHelper.ts (1)

5-12: Clarify contract between store and setSetting for status persistence

ModelStatusHelper reads from store:

const status = this.store.get(statusKey) as boolean | undefined

but writes via the injected setSetting function:

this.setSetting(statusKey, enabled)

while deleteModelStatus uses this.store.delete(statusKey).

This works if and only if setSetting ultimately writes into the same ElectronStore instance passed as store (e.g., setSetting = store.set.bind(store)). If setSetting is wired to a different store or abstraction, reads and writes could diverge.

It would be good to:

• Document that setSetting must target the same storage backend as store, or
• Simplify by using this.store.set(...) directly inside setModelStatus for symmetry with getModelStatus/deleteModelStatus.

Also applies to: 68-72, 108-112

src/renderer/src/stores/providerStore.ts (1)

199-223: optimizeProviderOrder ignores the enable flag; simplify or use it meaningfully

In optimizeProviderOrder, both branches of the ternary build the same newOrder:

const newOrder = enable
  ? [...enabledOrder, providerId, ...disabledOrder]
  : [...enabledOrder, providerId, ...disabledOrder]

So the enable parameter currently has no effect on ordering. Either:

• This is intentional and enable can be dropped from the function signature, or
• You meant to position the provider differently when disabling (e.g. append to the disabled section).

Clarifying this and removing the redundant ternary (or implementing distinct behavior per branch) would make the intent clearer and avoid confusion for future readers.

src/main/presenter/configPresenter/providerHelper.ts (2)

118-127: Consider making providerId optional or using a sentinel value for reorder operations.

The reorder operation sets providerId: '', which is semantically unclear since reordering affects the entire provider list rather than a single provider. Consumers of the PROVIDER_ATOMIC_UPDATE event might be confused by an empty string.

Consider either:

1. Making providerId optional in the ProviderChange interface for operations like reorder that don't target a single provider
2. Using a well-documented sentinel value (e.g., '__ALL__' or '*')
3. Creating a separate event type for reorder operations

---

51-60: Improve error handling in setProviderById.

When a provider is not found, the method only logs an error to the console but doesn't throw or return an error indication. This makes it difficult for callers to detect and handle failures.

Consider one of these approaches:

 setProviderById(id: string, provider: LLM_PROVIDER): void {
   const providers = this.getProviders()
   const index = providers.findIndex((p) => p.id === id)
   if (index !== -1) {
     providers[index] = provider
     this.setProviders(providers)
   } else {
-    console.error(`[Config] Provider ${id} not found`)
+    throw new Error(`[Config] Provider ${id} not found`)
   }
 }

Or return a boolean:

-setProviderById(id: string, provider: LLM_PROVIDER): void {
+setProviderById(id: string, provider: LLM_PROVIDER): boolean {
   const providers = this.getProviders()
   const index = providers.findIndex((p) => p.id === id)
   if (index !== -1) {
     providers[index] = provider
     this.setProviders(providers)
+    return true
   } else {
     console.error(`[Config] Provider ${id} not found`)
+    return false
   }
 }

src/renderer/settings/components/ProviderModelList.vue (1)

142-143: Add basic error handling around async modelStore operations

The switch to modelStore looks correct, but the async calls aren’t guarded:

• handleModelEnabledChange awaits modelStore.updateModelStatus without a try/catch, so any rejection becomes an unhandled promise and there’s no user feedback.
• If enableAllModels / disableAllModels are async in the store, they’ll also silently fail on rejection.

Consider wrapping these in try/catch and at least logging, to match the pattern in confirmAdd/handleDeleteCustomModel:

-const handleModelEnabledChange = async (model: RENDERER_MODEL_META, enabled: boolean) => {
-  emit('enabledChange', model, enabled)
-  await modelStore.updateModelStatus(model.providerId, model.id, enabled)
-}
+const handleModelEnabledChange = async (model: RENDERER_MODEL_META, enabled: boolean) => {
+  emit('enabledChange', model, enabled)
+  try {
+    await modelStore.updateModelStatus(model.providerId, model.id, enabled)
+  } catch (error) {
+    console.error('Failed to update model status:', error)
+  }
+}

And, if enableAllModels / disableAllModels are async, you might mirror that pattern there as well.

Also applies to: 155-156, 233-249, 251-272

src/renderer/src/stores/systemPromptStore.ts (1)

18-27: Ensure store state stays in sync after default/clear/save operations

addSystemPrompt, updateSystemPrompt, deleteSystemPrompt, and setDefaultSystemPromptId all reload via loadPrompts, but:

• setDefaultSystemPrompt
• resetToDefaultPrompt
• clearSystemPrompt
• (and arguably savePrompts)

don’t update prompts / defaultPromptId locally after calling the presenter. That can leave the store’s reactive state out of sync with persisted state until someone calls loadPrompts() elsewhere.

To keep consumers consistent, consider either:

• Calling await loadPrompts() at the end of these methods, or
• Updating prompts / defaultPromptId directly in these methods in parallel with the presenter calls.

Example:

const setDefaultSystemPrompt = async (content: string) => {
  await configP.setDefaultSystemPrompt(content)
+ await loadPrompts()
}

const resetToDefaultPrompt = async () => {
  await configP.resetToDefaultPrompt()
+ await loadPrompts()
}

const clearSystemPrompt = async () => {
  await configP.clearSystemPrompt()
+ await loadPrompts()
}

This keeps the store as the single source of truth for the UI.

Also applies to: 28-39, 40-59

src/renderer/src/views/WelcomeView.vue (1)

5-7: Provider/model store migration looks correct; consider reusing computed models and guarding final navigation

The migration here generally looks good:

• providerStore.providers is now the single source for provider metadata, and step‑1 logic correctly derives isOllamaApi from it.
• Models are pulled from modelStore.allProviderModels, and handleModelEnabledChange now updates model status through modelStore.updateModelStatus, which is consistent with the new store architecture.
• initAppStores() in onMounted gives you providers/models populated before the user proceeds through the steps.

Two small follow‑ups you might consider:

1. Reuse the providerModels computed in the template

You already have:

const providerModels = computed(() => {
  return (
    modelStore.allProviderModels.find((p) => p.providerId === selectedProvider.value)?.models ?? []
  )
})

But the template’s v‑for recomputes the same find:

<ModelConfigItem
  v-for="model in modelStore.allProviderModels.find(
    (p) => p.providerId === selectedProvider
  )?.models"
  ...
/>

For clarity and to avoid duplication, you can just loop over providerModels:

-                <ModelConfigItem
-                  v-for="model in modelStore.allProviderModels.find(
-                    (p) => p.providerId === selectedProvider
-                  )?.models"
+                <ModelConfigItem
+                  v-for="model in providerModels"
                   :key="model.id"
                   ...
                 />

2. Defensively handle missing models on final navigation

In the final step, you access providerModels.value[0].id without a length check. If model loading fails or returns an empty list, this will throw:

router.push({
  name: 'chat',
  query: {
    modelId: providerModels.value[0].id,
    providerId: selectedProvider.value
  }
})

A simple guard (fallback to a default model or showing an error dialog) would make the flow more robust.

Overall, the store refactor in this view looks solid.

Also applies to: 80-82, 121-130, 141-158, 183-209, 211-214, 216-223, 271-331, 363-375

src/renderer/src/lib/storeInitializer.ts (1)

34-69: Guard Electron IPC access in MCP deeplink handler for non‑Electron contexts

useMcpInstallDeeplinkHandler assumes window.electron.ipcRenderer is always available:

window.electron.ipcRenderer.on(DEEPLINK_EVENTS.MCP_INSTALL, handleMcpInstall)
...
window.electron.ipcRenderer.removeAllListeners(DEEPLINK_EVENTS.MCP_INSTALL)

This will throw in tests, Storybook, or any non‑Electron/web builds where window.electron is absent, unlike other code paths that use optional chaining.

To make it safer and consistent with patterns used elsewhere:

const setup = () => {
-  window.electron.ipcRenderer.on(DEEPLINK_EVENTS.MCP_INSTALL, handleMcpInstall)
+  if (!window?.electron?.ipcRenderer) return
+  window.electron.ipcRenderer.on(DEEPLINK_EVENTS.MCP_INSTALL, handleMcpInstall)
}

const cleanup = () => {
-  window.electron.ipcRenderer.removeAllListeners(DEEPLINK_EVENTS.MCP_INSTALL)
+  if (!window?.electron?.ipcRenderer) return
+  window.electron.ipcRenderer.removeAllListeners(DEEPLINK_EVENTS.MCP_INSTALL)
}

This keeps the deeplink behavior intact in Electron while avoiding crashes in other environments.

src/renderer/settings/components/AnthropicProviderSettingsDetail.vue (1)

367-457: Store migration looks correct; consider tightening provider auth updates and error surfacing

The migration from the old settings store to useProviderStore/useModelStore is consistent:

• Computeds (enabledModels, totalModelsCount, providerModels, customModels) correctly guard against missing provider entries with || [] / || 0.
• Auth-mode transitions use providerStore.updateProviderAuth in detectAuthMethod, switchAuthMethod, submitOAuthCode, and disconnectOAuth, which keeps the persisted state in sync.
• Model actions (disableAllModels, updateModelStatus) and provider removal (removeProvider) use the new stores as intended.

Two small follow-ups you may want to consider:

1. detectAuthMethod calls updateProviderAuth three times in the fallback path with undefined for the credential; if the helper already defaults the third argument, you could omit it or centralize that logic to avoid redundant writes.
2. All provider/model mutations are wrapped in try/catch with console.error only; if you want better UX, wiring these into the existing toast/error pipeline would give users feedback when provider operations fail.

If you’d like, I can draft a small helper to encapsulate the “detect auth + persist” flow, or hook these errors into your toast system. Please also double‑check that updateProviderAuth and updateProviderApi are defined with optional parameters as used here in providerStore. Based on learnings.

Also applies to: 471-487, 551-594, 610-618, 620-632, 648-652, 686-698, 709-712

src/renderer/settings/App.vue (1)

60-61: Settings window store initialization and UI store migration look sound

The changes here are coherent:

• initializeSettingsStores sequences providerStore.initialize(), modelStore.initialize(), optional ollamaStore.initialize?.(), and the search assistant/engine setup, with a catch‑all console.error to avoid breaking the mount flow.
• Using void initializeSettingsStores() in onMounted is appropriate since errors are handled inside.
• UI font size now consistently comes from uiSettingsStore.fontSizeClass and is applied via the watcher, while uiSettingsStore.loadSettings() is kicked off on mount.

No correctness issues stand out. The only optional tweak would be to document that initializeSettingsStores is intentionally fire‑and‑forget and safe to call once at settings window startup.

Please confirm that each of the called store methods (initialize, initOrUpdateSearchAssistantModel, refreshSearchEngines, setupSearchEnginesListener, loadSettings) are idempotent or guarded against multiple invocations, since this function may be reused from other entry points in the future.

Also applies to: 72-92, 117-145, 147-158, 195-199, 256-267

src/renderer/src/App.vue (1)

12-13: App‑level UI settings and store/deeplink initialization are wired correctly; watch body/html class usage

Key points in this refactor:

• Replacing the legacy settings store with useUiSettingsStore for font sizing and using uiSettingsStore.fontSizeLevel + updateFontSizeLevel in the zoom handlers is consistent with the new UI‑settings abstraction.
• The watcher on [themeStore.themeMode, uiSettingsStore.fontSizeClass] correctly keeps document.documentElement in sync with theme and font size, while initial classes are applied in onMounted.
• initAppStores() and useMcpInstallDeeplinkHandler() are integrated cleanly: init is invoked once on mount, and cleanupMcpDeeplink() is called during unmount alongside the IPC listener cleanup.

Two small things to keep an eye on:

1. The initial theme/font-size classes are added to document.body, but the watcher maintains them on document.documentElement. If your CSS expects them consistently on <html>, you might want to normalize that for the initial state too.
2. Zoom handlers don’t clamp the font size locally; this relies on uiSettingsStore.updateFontSizeLevel to validate the bounds. If that’s not already enforced, adding clamping there would prevent extreme values.

Please confirm that updateFontSizeLevel enforces valid bounds and that your global styles read theme/font-size from the same element (html vs body) for both initial and updated states.

Also applies to: 22-23, 30-31, 45-65, 147-163, 195-207, 313-332

src/renderer/src/stores/searchAssistantStore.ts (1)

1-141: New search assistant store is well‑structured; consider tightening validation and error handling

The useSearchAssistantStore design looks solid:

• State and computeds (modelRef/providerRef, searchAssistantModel, searchAssistantProvider) are straightforward.
• findPriorityModel correctly prioritizes models based on the priorities list and falls back to the first enabled Chat/ImageGeneration model with a provider.
• setSearchAssistantModel keeps Pinia state, config (configP.setSetting), and threadPresenter in sync.
• initOrUpdateSearchAssistantModel and checkAndUpdateSearchAssistantModel ensure there’s always a valid assistant model, reacting to changes in enabledModels via the deep watch.

A couple of robustness tweaks you might want to add:

1. Config presenter error handling
   Both setSearchAssistantModel and initOrUpdateSearchAssistantModel assume configP.getSetting/setSetting succeed. If these can throw or reject (e.g., disk issues), wrapping them in try/catch will prevent store consumers from seeing unhandled rejections.

2. Model existence check could also include providerId
   In checkAndUpdateSearchAssistantModel, you only check model.id. If there’s any chance of id collisions across providers or a model moving providers, you could tighten this to also match providerId:

   const stillAvailable = enabledModels.value.some((provider) =>
     provider.providerId === providerRef.value &&
     provider.models.some((model) => model.id === currentModel.id)
   )

   This keeps the selection aligned with both model and provider.

3. Optional: avoid re‑normalizing when using saved config
   For the saved case, you directly trust the stored payload, which is fine. If you later add schema changes, you may want a small normalization step similar to the one you use for the priority‑selected path.

Overall, this store is a good fit for the new single‑responsibility architecture; the above are just to harden it for edge cases.

Please confirm how configPresenter.getSetting/setSetting behave on failure, and whether model ids are guaranteed unique across providers; that will determine how necessary the stricter checks are here.

src/renderer/src/components/NewThread.vue (1)

149-149: Consider keeping artifacts in sync with UI settings

artifacts is initialized from uiSettingsStore.artifactsEffectEnabled but never updated if that setting changes while this component is mounted. If users can toggle this globally from elsewhere, adding a small watch on uiSettingsStore.artifactsEffectEnabled to update artifacts.value would avoid subtle desyncs.

src/renderer/src/stores/modelConfigStore.ts (1)

1-59: Model config store is well‑factored; consider cache invalidation hooks

The store cleanly wraps configPresenter with a simple per‑(provider,model) cache and covers the common operations (get/set/reset, provider listing, import/export). Implementation is correct and idiomatic for Pinia.

Two optional improvements you might consider:

• Cache invalidation: cache is only updated by getModelConfig/setModelConfig/resetModelConfig. If other parts of the app still call configPresenter.setModelConfig/resetModelConfig/importModelConfigs directly, this store could serve stale configs. Subscribing to CONFIG_EVENTS.MODEL_CONFIG_CHANGED / MODEL_CONFIG_RESET / MODEL_CONFIGS_IMPORTED in this store and clearing or updating relevant cache entries would make it more robust.
• Default provider cache key: Using 'default' in getCacheKey for missing providerId is fine, but if a real provider with id 'default' ever appears, keys could collide. A slightly more defensive prefix like '__default__' would avoid that edge case.

src/renderer/settings/components/prompt/SystemPromptSettingsSection.vue (1)

241-274: Reset/create/update/delete flows are consistent with centralized system prompt management

The reset, delete, and create/update handlers now all call the appropriate systemPromptStore methods (updateSystemPrompt, deleteSystemPrompt, addSystemPrompt, setDefaultSystemPromptId) and then refresh via loadSystemPrompts(). That keeps this UI thin and avoids duplicating persistence logic.

As a minor improvement, you could consider reusing the shared SystemPrompt type from @shared/presenter instead of SystemPromptItem, to avoid drift if shape changes, but that’s cosmetic.

Also applies to: 276-291, 305-335

src/renderer/settings/components/BedrockProviderSettingsDetail.vue (1)

250-259: Model status operations correctly use modelStore; watcher is a possible optimization

• Validation now calls providerStore.checkProvider and then modelStore.refreshProviderModels, which fits the new split of concerns.
• Enabling/disabling a single model and disabling all models now use modelStore.updateModelStatus and modelStore.disableAllModels, matching store responsibilities and keeping status mutations centralized.

As a minor optimization, the deep watcher on modelStore.allProviderModels could potentially be relaxed (e.g., watching a shallow version or an explicit version key) if this array becomes large, but the current approach is functionally correct.

Also applies to: 271-274, 276-283, 293-303, 309-316

src/renderer/src/stores/mcp.ts (2)

415-427: setMcpEnabled flow is robust but has some duplicated refresh responsibilities

The overall sequence—optimistic local toggle, main-process mutation, forced config refetch, server startup/status sync, and then forced tool/client/prompt reloads—is sensible and avoids the UI lag that existed before. Two points to consider:

• Some responsibilities overlap with syncConfigFromQuery (both can clear state and refresh queries on enable/disable), which makes the flow a bit harder to reason about when changes are driven from other windows vs this one.
• The extra delayed setTimeout refresh after enabling is defensively correct but adds complexity; if backends guarantee tool registration by the time updateAllServerStatuses completes, this could be simplified.

Neither is a correctness bug, but factoring the shared “after enable/disable” behavior into a single helper (used by both setMcpEnabled and syncConfigFromQuery) would make the flow easier to maintain.

Also applies to: 429-481

---

485-497: Server status updates now respect global mcpEnabled flag

Gating updateAllServerStatuses/updateServerStatus on config.value.mcpEnabled and clearing statuses when disabled aligns the UI with the global toggle and avoids pointless IPC when MCP is off. The logic that merges server-specific tools into enabledMcpTools when a server is running, and prunes invalid tools when it stops, reads correctly and should keep chat config in sync.

Only minor note: with global MCP disabled it’s still technically possible to start/stop servers via toggleServer, but updateServerStatus will treat them as not running. If that’s intentional (global off hides server state), this is fine; otherwise you may want to guard toggleServer on mcpEnabled in the UI.

Also applies to: 500-505, 518-524

src/main/presenter/configPresenter/uiSettingsHelper.ts (1)

1-69: UiSettingsHelper encapsulation and event emission look correct; consider harmonizing getter shapes

The helper cleanly wraps getSetting/setSetting with strongly named methods and emits the appropriate CONFIG_EVENTS.* to all windows on mutation, which matches the intended architecture. Two minor design notes:

• getSearchPreviewEnabled is asynchronous (Promise<boolean>) while the other getters are synchronous. If this isn’t constrained by existing callers, you might consider aligning them to a single style (all sync or all async) for consistency.
• Default behaviors (false for content protection / copy-with-COT, true for notifications when unset) seem reasonable, but worth double-checking against previous defaults to avoid surprising users on upgrade.

Functionally, though, the implementation is sound.

src/renderer/settings/components/common/SearchEngineSettingsSection.vue (2)

207-212: currentEngine computed works, but could be simplified

The currentEngine computed correctly resolves the selected engine from searchEngineStore.searchEngines and falls back to null. You could slightly simplify it by omitting the explicit || null since find already returns SearchEngineTemplate | undefined, but that’s purely cosmetic.

---

229-262: Custom engine add/delete flows correctly delegate to store but duplicate setSearchEngine calls

The refactor from mutating settingsStore.searchEngines in-place to:

• updating custom engines via configPresenter.setCustomSearchEngines(...), then
• await searchEngineStore.refreshSearchEngines(), and
• finally await searchEngineStore.setSearchEngine(...)

is a solid separation of concerns.

However, because you also assign selectedSearchEngine.value and have a watch(selectedSearchEngine, async ...) that calls searchEngineStore.setSearchEngine(newValue), each path ends up invoking setSearchEngine twice for the same id (once via the watcher, once explicitly). It’s harmless but unnecessary work.

You could rely on the watcher alone by removing the explicit setSearchEngine calls in addCustomSearchEngine and deleteCustomSearchEngine once selectedSearchEngine is updated.

Also applies to: 278-303

src/renderer/settings/components/ModelProviderSettings.vue (2)

217-219: Drag-and-drop reordering now delegates to providerStore appropriately

Using providerStore.sortedProviders as the base list for allEnabledProviders/allDisabledProviders, and calling providerStore.updateProvidersOrder(allProviders) in the computed setters, preserves the previous reordering behavior with the new store. The filtered-vs-unfiltered handling to maintain correct indices during search is careful and reads correctly.

If updateProvidersOrder is asynchronous, you might eventually want to make these setters async and await the call, but given likely synchronous store behavior this is not a blocking issue.

Also applies to: 221-239, 241-259

---

294-296: Active provider resolution now based on providerStore.providers

Using providerStore.providers.find((p) => p.id === route.params.providerId) is the right source of truth post-refactor. If you ever see inconsistent behavior when providerId can be an array, you may want to normalize it to String(route.params.providerId), but for normal routing this should be fine.

src/main/presenter/configPresenter/systemPromptHelper.ts (5)

8-11: DEFAULT_SYSTEM_PROMPT string has a minor typo

The last sentence is missing a space: "languages.Always" → "languages. Always". Consider fixing to avoid shipping this typo in the default text.

---

6-7: Confirm getSetting/setSetting contracts align with presenter implementations

GetSetting/SetSetting are typed as synchronous functions returning values directly. If configPresenter.getSetting/setSetting are Promise‑based (as in the renderer stores), passing them here would violate the contract and could lead to subtle bugs.

Either:

• Keep these types strictly sync and ensure callers pass sync wrappers, or
• Widen the types to accept async functions and await them in this helper.

Please double‑check the actual presenter signatures and adjust if needed.

Also applies to: 13-19

---

32-39: Consider falling back to DEFAULT_SYSTEM_PROMPT when no default is set

getDefaultSystemPrompt returns '' when there is no isDefault prompt and default_system_prompt isn’t set. Given you have a DEFAULT_SYSTEM_PROMPT constant and a resetToDefaultPrompt helper, it may be more user‑friendly to fall back to that constant instead of an empty string for a fresh install or missing setting.

Example:

-    return this.getSetting<string>('default_system_prompt') || ''
+    return this.getSetting<string>('default_system_prompt') || DEFAULT_SYSTEM_PROMPT

If an empty default is intentional (to explicitly mean “no system prompt”), keeping the current behavior is fine, but it’s worth validating.

---

41-51: Align default prompt mutations with DEFAULT_SYSTEM_PROMPT_CHANGED eventing

Right now, only setDefaultSystemPromptId emits CONFIG_EVENTS.DEFAULT_SYSTEM_PROMPT_CHANGED. Direct calls to setDefaultSystemPrompt, resetToDefaultPrompt, or clearSystemPrompt will update settings but won’t notify renderer windows.

If any callers mutate the default prompt via these methods without going through setDefaultSystemPromptId, UIs depending on the event may not update.

Consider either:

• Emitting the event from these methods as well, or
• Making setDefaultSystemPromptId the single public entry point for default changes and keeping the others private/internal.

Please verify how these methods are used in configPresenter and the renderer before deciding.

Also applies to: 87-113

---

115-128: Edge case when default prompt id is missing in stored prompts

When there is a non‑empty default_system_prompt but no prompt marked isDefault, getDefaultSystemPromptId falls back to 'default' even if no prompt with id 'default' actually exists.

If the intention is to represent a “legacy inline default” that isn’t part of the saved prompt list, this is fine. Otherwise, you may want to:

• Create/ensure a corresponding 'default' prompt in the list, or
• Return 'empty' (or null) to explicitly indicate “no matching prompt in the list”.

Worth double‑checking what the renderer expects when given 'default' but no such prompt exists.

src/renderer/src/stores/modelStore.ts (6)

93-127: Leverage modelConfigStore consistently (and ensure getModelConfig contract)

applyUserDefinedModelConfig calls modelConfigStore.getModelConfig(model.id, providerId) and assumes it may return null and an isUserDefined flag. That aligns with the snippet of useModelConfigStore, but make sure:

• The underlying presenter really can return configs that are not user‑defined (so isUserDefined is meaningful).
• getModelConfig is typed to allow the null case; otherwise TS will complain.

Functionally this is fine; this comment is mainly about keeping type signatures consistent with how you’re using them here.

---

129-161: Guard updateLocalModelStatus when the model isn’t yet in local state

updateLocalModelStatus assumes the model exists in allProviderModels or customModels. If an IPC MODEL_STATUS_CHANGED event arrives before models are hydrated for that provider, providerModel/customModel will be undefined and there may be no existing enabled group. In that case:

• You create an enabledProvider with an empty models array,
• But sourceModel is undefined, so nothing is pushed,
• enabledModels ends up containing a provider entry with an empty models list.

This doesn’t break, but it’s a slightly inconsistent state and may confuse consumers.

Consider early‑returning when you cannot find the model in any local collection, or triggering a refreshProviderModels(providerId) in that case instead of trying to mutate local state.

---

197-304: refreshStandardModels logic looks solid; just confirm Ollama interaction

The merge between DB models, stored models, and remote llmP.getModelList is careful and preserves overrides; getBatchModelStatus + applyUserDefinedModelConfig are used consistently.

The only thing to double‑check is how this behaves for providers with apiType === 'ollama', given the new ollamaStore also manipulates allProviderModels/enabledModels. Make sure there isn’t a tug‑of‑war between refreshStandardModels and syncOllamaModelsToGlobal over the same provider’s model list (whichever writes last “wins”).

---

417-435: updateModelStatus: optimistic update and Ollama special case

The optimistic update + rollback on error pattern is good. Note that for apiType === 'ollama' you mutate local state via updateLocalModelStatus but immediately return and never call llmP.updateModelStatus or refresh.

If Ollama statuses are intended to be managed solely by ollamaStore (and not persisted via llmP.updateModelStatus), this is fine; otherwise you might need symmetric handling here as well.

Worth quickly validating against how Ollama models are supposed to be enabled/disabled end‑to‑end.

---

437-481: Custom model helpers vs IPC mutations: ensure state stays in sync

You expose both:

• Imperative helpers (addCustomModel, removeCustomModel, updateCustomModel) that call llmP/configP and then refreshCustomModels, and
• IPC mutations (addCustomModelMutation, removeCustomModelMutation, updateCustomModelMutation) that only invalidate Colada queries.

Because customModels/allProviderModels/enabledModels are only updated in refreshCustomModels/refreshStandardModels, invalidating queries alone won’t update these refs unless some other path triggers a refresh (e.g., a CONFIG_EVENTS.MODEL_LIST_CHANGED handler).

Please confirm that:

• UI code uses one approach consistently (helpers or mutations), and
• Using the mutation path will always result in a subsequent refresh of the modelStore state (via events or explicit calls), so the internal refs never drift from the query data.

---

533-564: findModelByIdOrName only searches enabled models and has redundant id search

findModelByIdOrName currently:

1. Searches enabledModels for an exact id match,
2. Flattens enabledModels and searches by id again (redundant with step 1),
3. Falls back to a substring match on id/name, still only within enabled models.

If callers expect to be able to resolve disabled models too, this should probably also look into allProviderModels. If the intent is explicitly “search only among enabled models”, the behavior is OK, but you can drop the second exact‑id search to simplify the function.

Example simplification:

-    for (const providerModels of enabledModels.value) {
-      const model = providerModels.models.find((m) => m.id === modelId)
-      if (model) {
-        return { model, providerId: providerModels.providerId }
-      }
-    }
-
-    const enabledModel = enabledModels.value
-      .flatMap((provider) =>
-        provider.models.map((m) => ({ ...m, providerId: provider.providerId }))
-      )
-      .find((m) => m.id === modelId)
-    if (enabledModel) {
-      return { model: enabledModel, providerId: enabledModel.providerId! }
-    }
+    for (const providerModels of enabledModels.value) {
+      const model = providerModels.models.find((m) => m.id === modelId)
+      if (model) {
+        return { model, providerId: providerModels.providerId }
+      }
+    }

Optionally extend it to allProviderModels if needed.

src/renderer/src/stores/ollamaStore.ts (3)

62-71: Ollama model normalization is solid; consider reusing shared config logic

The way you derive contextLength, maxTokens, type, and capabilities from both modelConfig and ollamaModel.capabilities is thorough and matches how other providers are handled.

Given modelStore.applyUserDefinedModelConfig already encapsulates similar logic, consider extracting shared normalization into a helper (or reuse the modelConfigStore there) to avoid subtle divergence over time between Ollama and non‑Ollama models. Not urgent, but worth thinking about.

Also applies to: 85-139

---

72-175: Syncing Ollama models overrides global models; ensure it doesn’t fight modelStore

syncOllamaModelsToGlobal replaces allProviderModels[providerId].models with ollamaModelsAsGlobal and recalculates enabledModels for that provider. At the same time, modelStore.refreshStandardModels and refreshCustomModels also manage those same refs.

For apiType === 'ollama' providers this might be intended (Ollama is the source of truth), but it does mean:

• Whichever runs last (modelStore vs ollamaStore) wins the model list for that provider.
• Any future changes to modelStore’s invariants around allProviderModels/enabledModels won’t automatically apply here.

Please confirm that:

• refreshStandardModels is either not used for Ollama providers, or its effects are intentionally overwritten, and
• There are no components relying on non‑Ollama metadata for Ollama providers that would be lost here.

---

270-278: initialize assumes providerStore.providers is already hydrated

initialize filters providerStore.providers for enabled Ollama providers and immediately loops to refreshOllamaModels. This is fine as long as the app’s store initialization sequence ensures providerStore.initialize() (or an equivalent) has completed before ollamaStore.initialize() is called.

Please confirm the call order in your initAppStores wiring so that providers isn’t empty here due to pending async initialization.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

package.json (1)

106-106: Optional: Reconsider exact version pinning for devDependencies.

Changing @lingual/i18n-check from ^0.8.12 (flexible) to 0.8.12 (exact) prevents automatic receipt of security patches and bug fixes. While the enriched summary mentions this ensures consistency during refactoring, exact pins can add maintenance overhead.

Unless there's a specific incompatibility or test fragility issue with newer patch versions, consider reverting to ^0.8.12 to balance consistency with flexibility.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro