Auto save settings #874

zeroliu · 2024-11-28T01:10:46Z

Goal: Enable Auto-Saving for Settings

This PR introduces auto-saving functionality for settings by leveraging the Jotai state management library.

Implementation Details:

Atomizing settings:

settings is now an atom, enabling managers and components to observe changes reactively.
A new global access method, getSettings(), provides read access to settings, allowing significant cleanup of constructors.
Adding subscribeToSettingsChange in managers that need to reset on settings change.

Enforcing Controlled Updates:

Direct updates to settings have been removed since such changes were not observable and failed to refresh dependent components.
Updates must now use setSettings or updateSetting, ensuring proper state management.
Additionally, getSettings() now returns a readonly value, enabling TypeScript to catch unintended direct modifications.

Simplified State Management:

The langChainParams field has been removed.
In its place, chatModel and chainType have been converted into separate atoms, as they are not general settings. These individual atoms allow components and managers to observe and modify them independently.
The rests are replaced with settings.

Notes:

Global Reactivity: Any change to a settings field triggers resets or re-renders for all managers and components that rely on settings. While this is acceptable in most cases, it may lead to inefficiencies in edge scenarios.
Future Improvements: Observing changes at a more granular level is feasible but introduces complexity. Unlike React’s dependency arrays, manually selecting keys to observe lacks systematic checks and can lead to errors.

zeroliu · 2024-11-29T00:25:33Z

src/LLMProviders/brevilabsClient.ts

    }
    return BrevilabsClient.instance;
  }

  private checkLicenseKey() {
-    if (!this.licenseKey) {
+    if (!getSettings().plusLicenseKey) {


This change ensures BrevilabsClient always uses the latest license key

zeroliu · 2024-11-29T00:34:34Z

src/LLMProviders/chainManager.ts

+    subscribeToChainTypeChange(() =>
+      this.setChain(getChainType(), {
+        debug: getSettings().debug,
+        refreshIndex:


This chunk of code re-implements reindex on chain type change

zeroliu · 2024-11-29T00:39:36Z

src/LLMProviders/chainManager.ts

-  ): void {
-    this.langChainParams[key] = value;
+    this.createChainWithNewModel();
+    subscribeToModelKeyChange(() => this.createChainWithNewModel());


chain needs to be recreated when model key or settings change.

Here it listens to all settings key changes, which is not necessary. But it will cover all edge cases in case someone adds new settings dependency in the future.

zeroliu · 2024-11-29T00:40:05Z

src/LLMProviders/chainManager.ts

      this.chatModelManager.setChatModel(customModel);
      // Must update the chatModel for chain because ChainFactory always
      // retrieves the old chain without the chatModel change if it exists!
      // Create a new chain with the new chatModel
-      this.createChain(this.getLangChainParams().chainType, {


createChain and setChain has no functionality differences

zeroliu · 2024-11-29T00:40:47Z

src/LLMProviders/chainManager.ts

@@ -138,30 +120,15 @@ export default class ChainManager {
        customModel = BUILTIN_CHAT_MODELS[0];
        newModelKey = customModel.name + "|" + customModel.provider;
      }
-      this.setLangChainParam("modelKey", newModelKey);


This is done within setChain

zeroliu · 2024-11-29T00:41:14Z

src/LLMProviders/chainManager.ts

@@ -174,10 +141,7 @@ export default class ChainManager {
    this.validateChainType(chainType);

    // Handle index refresh if needed
-    if (
-      options.refreshIndex &&
-      (chainType === ChainType.VAULT_QA_CHAIN || chainType === ChainType.COPILOT_PLUS_CHAIN)


This check is done when setting refreshIndex

zeroliu · 2024-11-29T00:41:29Z

src/LLMProviders/chainManager.ts

@@ -188,25 +152,14 @@ export default class ChainManager {

    switch (chainType) {
      case ChainType.LLM_CHAIN: {
-        // For initial load of the plugin
-        if (options.forceNewCreation) {


forceNewCreation is always true

zeroliu · 2024-11-29T00:44:26Z

src/LLMProviders/chatModelManager.ts

@@ -261,9 +246,7 @@ export default class ChatModelManager {

    const modelConfig = this.getModelConfig(model);

-    // MUST update it since chatModelManager is a singleton.
-    this.getLangChainParams().modelKey = modelKey;
-    new Notice(`Setting model: ${modelConfig.modelName}`);


I am removing this notice because buildModelMap is called on any settings value change, which can be annoying for it to show when unrelated settings values are set

zeroliu · 2024-11-29T00:46:18Z

src/VectorStoreManager.ts

@@ -72,12 +56,6 @@ class VectorStoreManager {
        console.error("Failed to initialize Copilot database:", error);
      });

-    // Initialize the rate limiter
-    VectorDBManager.initialize({


This is no longer needed

zeroliu · 2024-11-29T00:48:41Z

src/VectorStoreManager.ts

-    encryptionService: EncryptionService,
-    getLangChainParams: () => LangChainParams
-  ) {
+  constructor(app: App) {


no subscription to settings needed in this manager because all the methods always use the latest settings value

zeroliu · 2024-11-29T00:49:17Z

src/aiParams.ts

@@ -24,42 +55,11 @@ export interface ModelConfig {
  enableCors?: boolean;
 }

-export interface LangChainParams {


LangChainParams have been replaced with settings, modelKey and chainType.

zeroliu · 2024-11-29T00:50:09Z

src/aiState.ts

This is no longer needed as components are interacting with the hooks exposed from aiParams.ts

zeroliu · 2024-11-29T00:50:46Z

src/components/Chat.tsx

@@ -11,7 +11,7 @@ import { getAIResponse } from "@/langchainStream";
 import ChainManager from "@/LLMProviders/chainManager";
 import CopilotPlugin from "@/main";
 import { Mention } from "@/mentions/Mention";
-import { useSettingsValueContext } from "@/settings/contexts/SettingsValueContext";


settings has become global. We don't need settings value context any more

zeroliu · 2024-11-29T00:56:16Z

src/customPromptProcessor.ts

-      if (!usageStrategy) {
-        console.warn("PromptUsageStrategy not initialize");
-      }


This code was very fragile. Many callers do not have access a usage strategy instance. If they ever get called before main.ts, it will not work. With the settings refactoring, this class can instantiate its usageStrategy now.

zeroliu · 2024-11-29T00:57:34Z

src/customPromptProcessor.ts

@@ -60,13 +56,13 @@ export class CustomPromptProcessor {
    }

    // Clean up promptUsageTimestamps
-    this.usageStrategy?.removeUnusedPrompts(prompts.map((prompt) => prompt.title)).save();


save is done automatically when any setting value changes now

zeroliu · 2024-11-29T00:59:16Z

src/encryptionService.ts

 }

-export default class EncryptionService {


we don't need a class if all the classes are static and there is no class state.

zeroliu · 2024-11-29T01:00:20Z

src/main.ts


  async onload(): Promise<void> {
    await this.loadSettings();
+    this.settingsUnsubscriber = subscribeToSettingsChange(() => {
+      this.saveData(getSettings());


auto save on any settings value change

zeroliu · 2024-11-29T01:02:07Z

src/main.ts

@@ -379,17 +340,6 @@ export default class CopilotPlugin extends Plugin {
      },
    });

-    // Index vault to Copilot index on startup and after loading all commands


This is already done in VectorStoreManager post init job

zeroliu · 2024-11-29T01:15:43Z

src/main.ts

@@ -550,27 +506,6 @@ export default class CopilotPlugin extends Plugin {
    return Array.from(modelMap.values());
  }

-  mergeAllActiveModelsWithCoreModels(): void {


moved to settings/model.ts

zeroliu · 2024-11-29T01:24:48Z

src/utils.ts

@@ -276,24 +275,6 @@ export function areEmbeddingModelsSame(
  return model1 === model2;
 }

-export function sanitizeSettings(settings: CopilotSettings): CopilotSettings {


moved to settings/model

zeroliu · 2024-11-29T01:25:18Z

src/vectorDBManager.ts


  private static getRateLimiter(): RateLimiter {
-    if (!this.config) {


config will always be set now

zeroliu · 2024-11-29T01:46:40Z

src/LLMProviders/brevilabsClient.ts

@@ -98,13 +92,13 @@ export class BrevilabsClient {
      method,
      headers: {
        "Content-Type": "application/json",
-        Authorization: `Bearer ${this.licenseKey}`,
+        Authorization: `Bearer ${getDecryptedKey(getSettings().plusLicenseKey)}`,


Found an existing bug! plus license key was not working with encryption enabled

zeroliu · 2024-11-29T01:47:20Z

src/main.ts

@@ -507,8 +464,9 @@ export default class CopilotPlugin extends Plugin {
          active: true,
        });
      }
+    } else {
+      this.app.workspace.revealLeaf(leaves[0]);


Found an existing bug! Reveal leaf will throw if the leaf is newly created

zeroliu · 2024-11-30T00:28:51Z

src/components/modals/ConfirmModal.tsx

Created a base class for confirm modal

zeroliu · 2024-11-30T00:29:15Z

src/components/modals/NewChatConfirmModal.tsx

Migrate the existing modal to use the new confirm modal. Also refine the styling of this modal

zeroliu · 2024-11-30T00:30:28Z

src/settings/components/SettingsMain.tsx

+    <div style={{ display: "flex", flexDirection: "column", gap: "1rem" }}>
+      <h1 style={{ display: "flex", alignItems: "center", justifyContent: "space-between" }}>
+        Copilot Settings
+        <button onClick={() => new ResetSettingsConfirmModal(app, () => resetSettings()).open()}>


added a modal to confirm the change here. This button becomes more powerful than before and it can get annoying if people accidentally click on it and lose all of their keys

zeroliu marked this pull request as draft November 28, 2024 01:10

zeroliu changed the title ~~Auto save settings~~ [WIP] Auto save settings Nov 28, 2024

logancyang linked an issue Nov 28, 2024 that may be closed by this pull request

Autosave settings after settings value are changed #848

Open

zeroliu force-pushed the zero/auto-save-settings branch 3 times, most recently from 2528c14 to d6df88e Compare November 29, 2024 00:23