Fix aligning settings between fe and be (All-Hands-AI#863)

* fix: aligning settings between FE and BE. * apply black formatter and clean useless codes.
marghoobsuleman · Apr 7, 2024 · e52bf5a · e52bf5a
1 parent e878b0c
commit e52bf5a
Show file tree

Hide file tree

Showing 16 changed files with 300 additions and 262 deletions.
diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
@@ -7,11 +7,11 @@ import Errors from "./components/Errors";
 import SettingModal from "./components/SettingModal";
 import Terminal from "./components/Terminal";
 import Workspace from "./components/Workspace";
-import store, { RootState } from "./store";
-import { setInitialized } from "./state/globalSlice";
 import { fetchMsgTotal } from "./services/session";
 import LoadMessageModal from "./components/LoadMessageModal";
-import { ResFetchMsgTotal } from "./types/ResponseType";
+import { ResConfigurations, ResFetchMsgTotal } from "./types/ResponseType";
+import { fetchConfigurations, saveSettings } from "./services/settingsService";
+import { RootState } from "./store";
 
 interface Props {
   setSettingOpen: (isOpen: boolean) => void;
@@ -30,22 +30,38 @@ function LeftNav({ setSettingOpen }: Props): JSX.Element {
   );
 }
 
+// React.StrictMode will cause double rendering, use this to prevent it
+let initOnce = false;
+
 function App(): JSX.Element {
-  const { initialized } = useSelector((state: RootState) => state.global);
   const [settingOpen, setSettingOpen] = useState(false);
   const [loadMsgWarning, setLoadMsgWarning] = useState(false);
+  const settings = useSelector((state: RootState) => state.settings);
 
   useEffect(() => {
-    if (!initialized) {
-      fetchMsgTotal()
-        .then((data: ResFetchMsgTotal) => {
-          if (data.msg_total > 0) {
-            setLoadMsgWarning(true);
-          }
-          store.dispatch(setInitialized(true));
-        })
-        .catch();
-    }
+    if (initOnce) return;
+    initOnce = true;
+    // only fetch configurations in the first time
+    fetchConfigurations()
+      .then((data: ResConfigurations) => {
+        saveSettings(
+          Object.fromEntries(
+            Object.entries(data).map(([key, value]) => [key, String(value)]),
+          ),
+          Object.fromEntries(
+            Object.entries(settings).map(([key, value]) => [key, value]),
+          ),
+          true,
+        );
+      })
+      .catch();
+    fetchMsgTotal()
+      .then((data: ResFetchMsgTotal) => {
+        if (data.msg_total > 0) {
+          setLoadMsgWarning(true);
+        }
+      })
+      .catch();
   }, []);
 
   const handleCloseModal = () => {

diff --git a/frontend/src/components/SettingModal.tsx b/frontend/src/components/SettingModal.tsx
@@ -1,31 +1,31 @@
 import React, { useEffect, useState } from "react";
 import { useSelector } from "react-redux";
 import {
+  Autocomplete,
+  AutocompleteItem,
+  Button,
+  Input,
   Modal,
-  ModalContent,
-  ModalHeader,
   ModalBody,
+  ModalContent,
   ModalFooter,
-  Input,
-  Button,
-  Autocomplete,
-  AutocompleteItem,
+  ModalHeader,
   Select,
   SelectItem,
 } from "@nextui-org/react";
 import { KeyboardEvent } from "@react-types/shared/src/events";
 import { useTranslation } from "react-i18next";
 import {
-  INITIAL_AGENTS,
-  fetchModels,
   fetchAgents,
+  fetchModels,
+  INITIAL_AGENTS,
   INITIAL_MODELS,
   saveSettings,
-  getInitialModel,
 } from "../services/settingsService";
 import { RootState } from "../store";
 import { I18nKey } from "../i18n/declaration";
 import { AvailableLanguages } from "../i18n";
+import ArgConfigType from "../types/ConfigType";
 
 interface Props {
   isOpen: boolean;
@@ -39,21 +39,17 @@ const cachedAgents = JSON.parse(
   localStorage.getItem("supportedAgents") || "[]",
 );
 
-function SettingModal({ isOpen, onClose }: Props): JSX.Element {
-  const defModel = useSelector((state: RootState) => state.settings.model);
-  const [model, setModel] = useState(defModel);
-  const defAgent = useSelector((state: RootState) => state.settings.agent);
-  const [agent, setAgent] = useState(defAgent);
-  const defWorkspaceDirectory = useSelector(
-    (state: RootState) => state.settings.workspaceDirectory,
+function InnerSettingModal({ isOpen, onClose }: Props): JSX.Element {
+  const settings = useSelector((state: RootState) => state.settings);
+  const [model, setModel] = useState(settings[ArgConfigType.LLM_MODEL]);
+  const [inputModel, setInputModel] = useState(
+    settings[ArgConfigType.LLM_MODEL],
   );
+  const [agent, setAgent] = useState(settings[ArgConfigType.AGENT]);
   const [workspaceDirectory, setWorkspaceDirectory] = useState(
-    defWorkspaceDirectory,
+    settings[ArgConfigType.WORKSPACE_DIR],
   );
-  const defLanguage = useSelector(
-    (state: RootState) => state.settings.language,
-  );
-  const [language, setLanguage] = useState(defLanguage);
+  const [language, setLanguage] = useState(settings[ArgConfigType.LANGUAGE]);
 
   const { t } = useTranslation();
 
@@ -65,12 +61,6 @@ function SettingModal({ isOpen, onClose }: Props): JSX.Element {
   );
 
   useEffect(() => {
-    getInitialModel()
-      .then((initialModel) => {
-        setModel(initialModel);
-      })
-      .catch();
-
     fetchModels().then((fetchedModels) => {
       const sortedModels = fetchedModels.sort(); // Sorting the models alphabetically
       setSupportedModels(sortedModels);
@@ -85,10 +75,16 @@ function SettingModal({ isOpen, onClose }: Props): JSX.Element {
 
   const handleSaveCfg = () => {
     saveSettings(
-      { model, agent, workspaceDirectory, language },
-      model !== defModel &&
-        agent !== defAgent &&
-        workspaceDirectory !== defWorkspaceDirectory,
+      {
+        [ArgConfigType.LLM_MODEL]: model ?? inputModel,
+        [ArgConfigType.AGENT]: agent,
+        [ArgConfigType.WORKSPACE_DIR]: workspaceDirectory,
+        [ArgConfigType.LANGUAGE]: language,
+      },
+      Object.fromEntries(
+        Object.entries(settings).map(([key, value]) => [key, value]),
+      ),
+      false,
     );
     onClose();
   };
@@ -127,9 +123,10 @@ function SettingModal({ isOpen, onClose }: Props): JSX.Element {
               onSelectionChange={(key) => {
                 setModel(key as string);
               }}
+              onInputChange={(e) => setInputModel(e)}
               onKeyDown={(e: KeyboardEvent) => e.continuePropagation()}
               defaultFilter={customFilter}
-              defaultInputValue={model}
+              defaultInputValue={inputModel}
               allowsCustomValue
             >
               {(item: { label: string; value: string }) => (
@@ -187,4 +184,10 @@ function SettingModal({ isOpen, onClose }: Props): JSX.Element {
   );
 }
 
+function SettingModal({ isOpen, onClose }: Props): JSX.Element {
+  // Do not render the modal if it is not open, prevents reading empty from localStorage after initialization
+  if (!isOpen) return <div />;
+  return <InnerSettingModal isOpen={isOpen} onClose={onClose} />;
+}
+
 export default SettingModal;
diff --git a/frontend/src/i18n/index.ts b/frontend/src/i18n/index.ts
@@ -2,6 +2,7 @@ import i18n from "i18next";
 import Backend from "i18next-http-backend";
 import LanguageDetector from "i18next-browser-languagedetector";
 import { initReactI18next } from "react-i18next";
+import ArgConfigType from "../types/ConfigType";
 
 export const AvailableLanguages = [
   { label: "English", value: "en" },
@@ -21,14 +22,14 @@ i18n
     // assume all detected   languages are available
     const detectLanguage = i18n.language;
     // cannot trust browser language setting
-    const settingLanguage = localStorage.getItem("language");
+    const settingLanguage = localStorage.getItem(ArgConfigType.LANGUAGE);
 
     // if setting is not initialized, but detected language is available, use detected language and update language setting
     if (
       !settingLanguage &&
       AvailableLanguages.some((lang) => detectLanguage === lang.value)
     ) {
-      localStorage.setItem("language", detectLanguage);
+      localStorage.setItem(ArgConfigType.LANGUAGE, detectLanguage);
       i18n.changeLanguage(detectLanguage);
       return;
     }

diff --git a/frontend/src/services/settingsService.ts b/frontend/src/services/settingsService.ts
@@ -3,20 +3,20 @@ import { setInitialized } from "../state/taskSlice";
 import store from "../store";
 import ActionType from "../types/ActionType";
 import Socket from "./socket";
-import {
-  setAgent,
-  setLanguage,
-  setModel,
-  setWorkspaceDirectory,
-} from "../state/settingsSlice";
+import { setByKey } from "../state/settingsSlice";
+import { ResConfigurations } from "../types/ResponseType";
+import ArgConfigType from "../types/ConfigType";
 
-export async function getInitialModel() {
-  if (localStorage.getItem("model")) {
-    return localStorage.getItem("model");
+export async function fetchConfigurations(): Promise<ResConfigurations> {
+  const headers = new Headers({
+    "Content-Type": "application/json",
+    Authorization: `Bearer ${localStorage.getItem("token")}`,
+  });
+  const response = await fetch(`/api/configurations`, { headers });
+  if (response.status !== 200) {
+    throw new Error("Get configurations failed.");
   }
-
-  const res = await fetch("/api/default-model");
-  return res.json();
+  return (await response.json()) as ResConfigurations;
 }
 
 export async function fetchModels() {
@@ -43,36 +43,53 @@ export const INITIAL_AGENTS = ["MonologueAgent", "CodeActAgent"];
 
 export type Agent = (typeof INITIAL_AGENTS)[number];
 
-// Map Redux settings to socket event arguments
-const SETTINGS_MAP = new Map<string, string>([
-  ["model", "model"],
-  ["agent", "agent_cls"],
-  ["workspaceDirectory", "directory"],
+// TODO: add the values to i18n to support multi languages
+const DISPLAY_MAP = new Map<string, string>([
+  [ArgConfigType.LLM_MODEL, "model"],
+  [ArgConfigType.AGENT, "agent"],
+  [ArgConfigType.WORKSPACE_DIR, "directory"],
+  [ArgConfigType.LANGUAGE, "language"],
 ]);
 
 // Send settings to the server
 export function saveSettings(
-  reduxSettings: { [id: string]: string },
-  needToSend: boolean = false,
+  newSettings: { [key: string]: string },
+  oldSettings: { [key: string]: string },
+  isInit: boolean = false,
 ): void {
-  if (needToSend) {
-    const socketSettings = Object.fromEntries(
-      Object.entries(reduxSettings).map(([setting, value]) => [
-        SETTINGS_MAP.get(setting) || setting,
-        value,
-      ]),
-    );
-    const event = { action: ActionType.INIT, args: socketSettings };
+  let needToSend = false;
+  const updatedSettings: { [key: string]: string } = {};
+  const mergedSettings = { ...oldSettings, ...newSettings };
+  Object.keys(newSettings).forEach((key) => {
+    if (
+      Object.hasOwnProperty.call(oldSettings, key) &&
+      oldSettings[key] !== String(newSettings[key])
+    ) {
+      if (isInit && oldSettings[key] !== "") {
+        mergedSettings[key] = oldSettings[key];
+        return;
+      }
+      needToSend = true;
+      updatedSettings[key] = String(newSettings[key]);
+      mergedSettings[key] = String(newSettings[key]);
+    } else {
+      mergedSettings[key] = oldSettings[key];
+    }
+  });
+
+  if (needToSend || isInit) {
+    const event = { action: ActionType.INIT, args: mergedSettings };
     const eventString = JSON.stringify(event);
     store.dispatch(setInitialized(false));
     Socket.send(eventString);
   }
-  for (const [setting, value] of Object.entries(reduxSettings)) {
-    localStorage.setItem(setting, value);
-    store.dispatch(appendAssistantMessage(`Set ${setting} to "${value}"`));
+
+  for (const [key, value] of Object.entries(updatedSettings)) {
+    if (DISPLAY_MAP.has(key)) {
+      store.dispatch(setByKey({ key, value }));
+      store.dispatch(
+        appendAssistantMessage(`Set ${DISPLAY_MAP.get(key)} to "${value}"`),
+      );
+    }
   }
-  store.dispatch(setModel(reduxSettings.model));
-  store.dispatch(setAgent(reduxSettings.agent));
-  store.dispatch(setWorkspaceDirectory(reduxSettings.workspaceDirectory));
-  store.dispatch(setLanguage(reduxSettings.language));
 }
diff --git a/frontend/src/services/socket.ts b/frontend/src/services/socket.ts
@@ -2,7 +2,6 @@ import store from "../store";
 import { appendError, removeError } from "../state/errorsSlice";
 import { handleAssistantMessage } from "./actions";
 import { getToken } from "./auth";
-import ActionType from "../types/ActionType";
 
 class Socket {
   private static _socket: WebSocket | null = null;
@@ -26,22 +25,7 @@ class Socket {
       const WS_URL = `ws://${window.location.host}/ws?token=${token}`;
       Socket._socket = new WebSocket(WS_URL);
 
-      Socket._socket.onopen = () => {
-        const model = localStorage.getItem("model") || "gpt-3.5-turbo-1106";
-        const agent = localStorage.getItem("agent") || "MonologueAgent";
-        const workspaceDirectory =
-          localStorage.getItem("workspaceDirectory") || "./workspace";
-        Socket._socket?.send(
-          JSON.stringify({
-            action: ActionType.INIT,
-            args: {
-              model,
-              agent_cls: agent,
-              directory: workspaceDirectory,
-            },
-          }),
-        );
-      };
+      Socket._socket.onopen = () => {};
 
       Socket._socket.onmessage = (e) => {
         handleAssistantMessage(e.data);

diff --git a/frontend/src/state/globalSlice.ts b/frontend/src/state/globalSlice.ts