Просмотр исходного кода

Redesign settings modal (#3751)

* add advanced options switch

* remove logic for custom models

* remove extra span

* consolidate row

* no more toast

* remove default subtitle

* fix title tags

* remove getSettingsDifference

* delint

* delint

* fix select buttons

* delete test

* fix test

* fix test

* fix labels

* remove model-id tests

* fix labels

* fix labels

* rename var

* fix test

* fix more tests

* change some mocks

* more fixes

* one more fix

* remove some mocks

* rename things

* one more test

* ahhhhh

* change required settings

* fix test

* delint

* fix build
Robert Brennan 1 год назад
Родитель
Сommit
cb3168da12

+ 2 - 3
frontend/src/components/modals/base-modal/BaseModal.tsx

@@ -39,7 +39,6 @@ function BaseModal({
       data-testid={testID}
       isOpen={isOpen}
       onOpenChange={onOpenChange}
-      title={title}
       isDismissable={isDismissable}
       backdrop="blur"
       hideCloseButton
@@ -51,14 +50,14 @@ function BaseModal({
           <>
             {title && (
               <ModalHeader className="flex flex-col p-0">
-                <HeaderContent title={title} subtitle={subtitle} />
+                <HeaderContent maintitle={title} subtitle={subtitle} />
               </ModalHeader>
             )}
 
             <ModalBody className={bodyClassName}>{children}</ModalBody>
 
             {actions && actions.length > 0 && (
-              <ModalFooter className="flex-col flex justify-start p-0">
+              <ModalFooter className="flex-row flex justify-start p-0">
                 <FooterContent actions={actions} closeModal={closeModal} />
               </ModalFooter>
             )}

+ 3 - 3
frontend/src/components/modals/base-modal/HeaderContent.tsx

@@ -1,17 +1,17 @@
 import React from "react";
 
 interface HeaderContentProps {
-  title: string;
+  maintitle: string;
   subtitle?: string;
 }
 
 export function HeaderContent({
-  title,
+  maintitle,
   subtitle = undefined,
 }: HeaderContentProps) {
   return (
     <>
-      <h3>{title}</h3>
+      <h3>{maintitle}</h3>
       {subtitle && (
         <span className="text-neutral-400 text-sm font-light">{subtitle}</span>
       )}

+ 9 - 62
frontend/src/components/modals/settings/ModelSelector.test.tsx

@@ -29,7 +29,7 @@ describe("ModelSelector", () => {
     const onModelChange = vi.fn();
     render(<ModelSelector models={models} onModelChange={onModelChange} />);
 
-    const selector = screen.getByLabelText("Provider");
+    const selector = screen.getByLabelText("LLM Provider");
     expect(selector).toBeInTheDocument();
 
     await user.click(selector);
@@ -45,10 +45,10 @@ describe("ModelSelector", () => {
     const onModelChange = vi.fn();
     render(<ModelSelector models={models} onModelChange={onModelChange} />);
 
-    const modelSelector = screen.getByLabelText("Model");
+    const modelSelector = screen.getByLabelText("LLM Model");
     expect(modelSelector).toBeDisabled();
 
-    const providerSelector = screen.getByLabelText("Provider");
+    const providerSelector = screen.getByLabelText("LLM Provider");
     await user.click(providerSelector);
 
     const vertexAI = screen.getByText("VertexAI");
@@ -62,13 +62,13 @@ describe("ModelSelector", () => {
     const onModelChange = vi.fn();
     render(<ModelSelector models={models} onModelChange={onModelChange} />);
 
-    const providerSelector = screen.getByLabelText("Provider");
+    const providerSelector = screen.getByLabelText("LLM Provider");
     await user.click(providerSelector);
 
     const azureProvider = screen.getByText("Azure");
     await user.click(azureProvider);
 
-    const modelSelector = screen.getByLabelText("Model");
+    const modelSelector = screen.getByLabelText("LLM Model");
     await user.click(modelSelector);
 
     expect(screen.getByText("ada")).toBeInTheDocument();
@@ -84,42 +84,13 @@ describe("ModelSelector", () => {
     expect(screen.getByText("chat-bison-32k")).toBeInTheDocument();
   });
 
-  it("should display the actual litellm model ID as the user is making the selections", async () => {
-    const user = userEvent.setup();
-    const onModelChange = vi.fn();
-    render(<ModelSelector models={models} onModelChange={onModelChange} />);
-
-    const id = screen.getByTestId("model-id");
-    const providerSelector = screen.getByLabelText("Provider");
-    const modelSelector = screen.getByLabelText("Model");
-
-    expect(id).toHaveTextContent("No model selected");
-
-    await user.click(providerSelector);
-    await user.click(screen.getByText("Azure"));
-
-    expect(id).toHaveTextContent("azure/");
-
-    await user.click(modelSelector);
-    await user.click(screen.getByText("ada"));
-    expect(id).toHaveTextContent("azure/ada");
-
-    await user.click(providerSelector);
-    await user.click(screen.getByText("cohere"));
-    expect(id).toHaveTextContent("cohere.");
-
-    await user.click(modelSelector);
-    await user.click(screen.getByText("command-r-v1:0"));
-    expect(id).toHaveTextContent("cohere.command-r-v1:0");
-  });
-
   it("should call onModelChange when the model is changed", async () => {
     const user = userEvent.setup();
     const onModelChange = vi.fn();
     render(<ModelSelector models={models} onModelChange={onModelChange} />);
 
-    const providerSelector = screen.getByLabelText("Provider");
-    const modelSelector = screen.getByLabelText("Model");
+    const providerSelector = screen.getByLabelText("LLM Provider");
+    const modelSelector = screen.getByLabelText("LLM Model");
 
     await user.click(providerSelector);
     await user.click(screen.getByText("Azure"));
@@ -146,29 +117,6 @@ describe("ModelSelector", () => {
     expect(onModelChange).toHaveBeenCalledWith("cohere.command-r-v1:0");
   });
 
-  it("should clear the model ID when the provider is cleared", async () => {
-    const user = userEvent.setup();
-    const onModelChange = vi.fn();
-    render(<ModelSelector models={models} onModelChange={onModelChange} />);
-
-    const providerSelector = screen.getByLabelText("Provider");
-    const modelSelector = screen.getByLabelText("Model");
-
-    await user.click(providerSelector);
-    await user.click(screen.getByText("Azure"));
-
-    await user.click(modelSelector);
-    await user.click(screen.getByText("ada"));
-
-    expect(screen.getByTestId("model-id")).toHaveTextContent("azure/ada");
-
-    await user.clear(providerSelector);
-
-    expect(screen.getByTestId("model-id")).toHaveTextContent(
-      "No model selected",
-    );
-  });
-
   it("should have a default value if passed", async () => {
     const onModelChange = vi.fn();
     render(
@@ -179,9 +127,8 @@ describe("ModelSelector", () => {
       />,
     );
 
-    expect(screen.getByTestId("model-id")).toHaveTextContent("azure/ada");
-    expect(screen.getByLabelText("Provider")).toHaveValue("Azure");
-    expect(screen.getByLabelText("Model")).toHaveValue("ada");
+    expect(screen.getByLabelText("LLM Provider")).toHaveValue("Azure");
+    expect(screen.getByLabelText("LLM Model")).toHaveValue("ada");
   });
 
   it.todo("should disable provider if isDisabled is true");

+ 4 - 8
frontend/src/components/modals/settings/ModelSelector.tsx

@@ -21,7 +21,7 @@ export function ModelSelector({
   onModelChange,
   defaultModel,
 }: ModelSelectorProps) {
-  const [litellmId, setLitellmId] = React.useState<string | null>(null);
+  const [, setLitellmId] = React.useState<string | null>(null);
   const [selectedProvider, setSelectedProvider] = React.useState<string | null>(
     null,
   );
@@ -61,14 +61,10 @@ export function ModelSelector({
 
   return (
     <div data-testid="model-selector" className="flex flex-col gap-2">
-      <span className="text-center italic text-gray-500" data-testid="model-id">
-        {litellmId?.replace("other", "") || "No model selected"}
-      </span>
-
-      <div className="flex flex-col gap-3">
+      <div className="flex flex-row gap-3">
         <Autocomplete
           isDisabled={isDisabled}
-          label="Provider"
+          label="LLM Provider"
           placeholder="Select a provider"
           isClearable={false}
           onSelectionChange={(e) => {
@@ -99,7 +95,7 @@ export function ModelSelector({
         </Autocomplete>
 
         <Autocomplete
-          label="Model"
+          label="LLM Model"
           placeholder="Select a model"
           onSelectionChange={(e) => {
             if (e?.toString()) handleChangeModel(e.toString());

+ 28 - 60
frontend/src/components/modals/settings/SettingsForm.test.tsx

@@ -6,8 +6,6 @@ import { Settings } from "#/services/settings";
 import SettingsForm from "./SettingsForm";
 
 const onModelChangeMock = vi.fn();
-const onCustomModelChangeMock = vi.fn();
-const onModelTypeChangeMock = vi.fn();
 const onAgentChangeMock = vi.fn();
 const onLanguageChangeMock = vi.fn();
 const onAPIKeyChangeMock = vi.fn();
@@ -21,21 +19,17 @@ const renderSettingsForm = (settings?: Settings) => {
       settings={
         settings || {
           LLM_MODEL: "gpt-4o",
-          CUSTOM_LLM_MODEL: "",
-          USING_CUSTOM_MODEL: false,
           AGENT: "agent1",
           LANGUAGE: "en",
           LLM_API_KEY: "sk-...",
-          CONFIRMATION_MODE: true,
-          SECURITY_ANALYZER: "analyzer1",
+          CONFIRMATION_MODE: false,
+          SECURITY_ANALYZER: "",
         }
       }
       models={["gpt-4o", "gpt-3.5-turbo", "azure/ada"]}
       agents={["agent1", "agent2", "agent3"]}
       securityAnalyzers={["analyzer1", "analyzer2", "analyzer3"]}
       onModelChange={onModelChangeMock}
-      onCustomModelChange={onCustomModelChangeMock}
-      onModelTypeChange={onModelTypeChangeMock}
       onAgentChange={onAgentChangeMock}
       onLanguageChange={onLanguageChangeMock}
       onAPIKeyChange={onAPIKeyChangeMock}
@@ -49,50 +43,38 @@ describe("SettingsForm", () => {
   it("should display the first values in the array by default", () => {
     renderSettingsForm();
 
-    const providerInput = screen.getByRole("combobox", { name: "Provider" });
-    const modelInput = screen.getByRole("combobox", { name: "Model" });
-    const agentInput = screen.getByRole("combobox", { name: "agent" });
+    const providerInput = screen.getByRole("combobox", {
+      name: "LLM Provider",
+    });
+    const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
     const languageInput = screen.getByRole("combobox", { name: "language" });
     const apiKeyInput = screen.getByTestId("apikey");
-    const confirmationModeInput = screen.getByTestId("confirmationmode");
-    const securityAnalyzerInput = screen.getByRole("combobox", {
-      name: "securityanalyzer",
-    });
 
     expect(providerInput).toHaveValue("OpenAI");
     expect(modelInput).toHaveValue("gpt-4o");
-    expect(agentInput).toHaveValue("agent1");
     expect(languageInput).toHaveValue("English");
     expect(apiKeyInput).toHaveValue("sk-...");
-    expect(confirmationModeInput).toHaveAttribute("data-selected", "true");
-    expect(securityAnalyzerInput).toHaveValue("analyzer1");
   });
 
   it("should display the existing values if they are present", () => {
     renderSettingsForm({
       LLM_MODEL: "gpt-3.5-turbo",
-      CUSTOM_LLM_MODEL: "",
-      USING_CUSTOM_MODEL: false,
       AGENT: "agent2",
       LANGUAGE: "es",
       LLM_API_KEY: "sk-...",
-      CONFIRMATION_MODE: true,
-      SECURITY_ANALYZER: "analyzer2",
+      CONFIRMATION_MODE: false,
+      SECURITY_ANALYZER: "",
     });
 
-    const providerInput = screen.getByRole("combobox", { name: "Provider" });
-    const modelInput = screen.getByRole("combobox", { name: "Model" });
-    const agentInput = screen.getByRole("combobox", { name: "agent" });
-    const languageInput = screen.getByRole("combobox", { name: "language" });
-    const securityAnalyzerInput = screen.getByRole("combobox", {
-      name: "securityanalyzer",
+    const providerInput = screen.getByRole("combobox", {
+      name: "LLM Provider",
     });
+    const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
+    const languageInput = screen.getByRole("combobox", { name: "language" });
 
     expect(providerInput).toHaveValue("OpenAI");
     expect(modelInput).toHaveValue("gpt-3.5-turbo");
-    expect(agentInput).toHaveValue("agent2");
     expect(languageInput).toHaveValue("Español");
-    expect(securityAnalyzerInput).toHaveValue("analyzer2");
   });
 
   it("should disable settings when disabled is true", () => {
@@ -100,21 +82,17 @@ describe("SettingsForm", () => {
       <SettingsForm
         settings={{
           LLM_MODEL: "gpt-4o",
-          CUSTOM_LLM_MODEL: "",
-          USING_CUSTOM_MODEL: false,
           AGENT: "agent1",
           LANGUAGE: "en",
           LLM_API_KEY: "sk-...",
-          CONFIRMATION_MODE: true,
-          SECURITY_ANALYZER: "analyzer1",
+          CONFIRMATION_MODE: false,
+          SECURITY_ANALYZER: "",
         }}
         models={["gpt-4o", "gpt-3.5-turbo", "azure/ada"]}
         agents={["agent1", "agent2", "agent3"]}
         securityAnalyzers={["analyzer1", "analyzer2", "analyzer3"]}
         disabled
         onModelChange={onModelChangeMock}
-        onCustomModelChange={onCustomModelChangeMock}
-        onModelTypeChange={onModelTypeChangeMock}
         onAgentChange={onAgentChangeMock}
         onLanguageChange={onLanguageChangeMock}
         onAPIKeyChange={onAPIKeyChangeMock}
@@ -123,21 +101,15 @@ describe("SettingsForm", () => {
       />,
     );
 
-    const providerInput = screen.getByRole("combobox", { name: "Provider" });
-    const modelInput = screen.getByRole("combobox", { name: "Model" });
-    const agentInput = screen.getByRole("combobox", { name: "agent" });
-    const languageInput = screen.getByRole("combobox", { name: "language" });
-    const confirmationModeInput = screen.getByTestId("confirmationmode");
-    const securityAnalyzerInput = screen.getByRole("combobox", {
-      name: "securityanalyzer",
+    const providerInput = screen.getByRole("combobox", {
+      name: "LLM Provider",
     });
+    const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
+    const languageInput = screen.getByRole("combobox", { name: "language" });
 
     expect(providerInput).toBeDisabled();
     expect(modelInput).toBeDisabled();
-    expect(agentInput).toBeDisabled();
     expect(languageInput).toBeDisabled();
-    expect(confirmationModeInput).toHaveAttribute("data-disabled", "true");
-    expect(securityAnalyzerInput).toBeDisabled();
   });
 
   describe("onChange handlers", () => {
@@ -146,7 +118,7 @@ describe("SettingsForm", () => {
       renderSettingsForm();
 
       // We need to enable the agent select
-      const agentSwitch = screen.getByTestId("enableagentselect");
+      const agentSwitch = screen.getByTestId("advanced-options-toggle");
       await user.click(agentSwitch);
 
       const agentInput = screen.getByRole("combobox", { name: "agent" });
@@ -201,8 +173,8 @@ describe("SettingsForm", () => {
       const user = userEvent.setup();
       renderSettingsForm();
 
-      const customModelToggle = screen.getByTestId("custom-model-toggle");
-      await user.click(customModelToggle);
+      const advancedToggle = screen.getByTestId("advanced-options-toggle");
+      await user.click(advancedToggle);
 
       const modelSelector = screen.queryByTestId("model-selector");
       expect(modelSelector).not.toBeInTheDocument();
@@ -215,23 +187,21 @@ describe("SettingsForm", () => {
       const user = userEvent.setup();
       renderSettingsForm();
 
-      const customModelToggle = screen.getByTestId("custom-model-toggle");
-      await user.click(customModelToggle);
+      const advancedToggle = screen.getByTestId("advanced-options-toggle");
+      await user.click(advancedToggle);
 
       const customModelInput = screen.getByTestId("custom-model-input");
+      await userEvent.clear(customModelInput);
       await userEvent.type(customModelInput, "my/custom-model");
 
-      expect(onCustomModelChangeMock).toHaveBeenCalledWith("my/custom-model");
-      expect(onModelTypeChangeMock).toHaveBeenCalledWith("custom");
+      expect(onModelChangeMock).toHaveBeenCalledWith("my/custom-model");
     });
 
-    it("should have custom model switched if using custom model", () => {
+    it("should have advanced options switched if using advanced options", () => {
       renderWithProviders(
         <SettingsForm
           settings={{
             LLM_MODEL: "gpt-4o",
-            CUSTOM_LLM_MODEL: "CUSTOM_MODEL",
-            USING_CUSTOM_MODEL: true,
             AGENT: "agent1",
             LANGUAGE: "en",
             LLM_API_KEY: "sk-...",
@@ -243,8 +213,6 @@ describe("SettingsForm", () => {
           securityAnalyzers={["analyzer1", "analyzer2", "analyzer3"]}
           disabled
           onModelChange={onModelChangeMock}
-          onCustomModelChange={onCustomModelChangeMock}
-          onModelTypeChange={onModelTypeChangeMock}
           onAgentChange={onAgentChangeMock}
           onLanguageChange={onLanguageChangeMock}
           onAPIKeyChange={onAPIKeyChangeMock}
@@ -253,8 +221,8 @@ describe("SettingsForm", () => {
         />,
       );
 
-      const customModelToggle = screen.getByTestId("custom-model-toggle");
-      expect(customModelToggle).toHaveAttribute("aria-checked", "true");
+      const advancedToggle = screen.getByTestId("advanced-options-toggle");
+      expect(advancedToggle).toHaveAttribute("aria-checked", "true");
     });
   });
 });

+ 55 - 72
frontend/src/components/modals/settings/SettingsForm.tsx

@@ -17,8 +17,6 @@ interface SettingsFormProps {
   disabled: boolean;
 
   onModelChange: (model: string) => void;
-  onCustomModelChange: (model: string) => void;
-  onModelTypeChange: (type: "custom" | "default") => void;
   onAPIKeyChange: (apiKey: string) => void;
   onAgentChange: (agent: string) => void;
   onLanguageChange: (language: string) => void;
@@ -33,8 +31,6 @@ function SettingsForm({
   securityAnalyzers,
   disabled,
   onModelChange,
-  onCustomModelChange,
-  onModelTypeChange,
   onAPIKeyChange,
   onAgentChange,
   onLanguageChange,
@@ -43,40 +39,31 @@ function SettingsForm({
 }: SettingsFormProps) {
   const { t } = useTranslation();
   const { isOpen: isVisible, onOpenChange: onVisibleChange } = useDisclosure();
-  const [isAgentSelectEnabled, setIsAgentSelectEnabled] = React.useState(false);
-  const [usingCustomModel, setUsingCustomModel] = React.useState(
-    settings.USING_CUSTOM_MODEL,
-  );
-
-  const changeModelType = (type: "custom" | "default") => {
-    if (type === "custom") {
-      setUsingCustomModel(true);
-      onModelTypeChange("custom");
-    } else {
-      setUsingCustomModel(false);
-      onModelTypeChange("default");
-    }
-  };
+  const advancedAlreadyInUse =
+    !!settings.SECURITY_ANALYZER || !!settings.CONFIRMATION_MODE;
+  // TODO: || model is not in the list
+  const [enableAdvanced, setEnableAdvanced] =
+    React.useState(advancedAlreadyInUse);
 
   return (
     <>
       <Switch
-        data-testid="custom-model-toggle"
-        aria-checked={usingCustomModel}
-        isSelected={usingCustomModel}
-        onValueChange={(value) => changeModelType(value ? "custom" : "default")}
+        data-testid="advanced-options-toggle"
+        aria-checked={enableAdvanced}
+        isSelected={enableAdvanced}
+        onValueChange={(value) => setEnableAdvanced(value)}
       >
-        Use custom model
+        Advanced Options
       </Switch>
-      {usingCustomModel && (
+      {enableAdvanced && (
         <Input
           data-testid="custom-model-input"
           label="Custom Model"
-          onValueChange={onCustomModelChange}
-          defaultValue={settings.CUSTOM_LLM_MODEL}
+          onValueChange={onModelChange}
+          defaultValue={settings.LLM_MODEL}
         />
       )}
-      {!usingCustomModel && (
+      {!enableAdvanced && (
         <ModelSelector
           isDisabled={disabled}
           models={organizeModelsAndProviders(models)}
@@ -117,52 +104,48 @@ function SettingsForm({
         tooltip={t(I18nKey.SETTINGS$LANGUAGE_TOOLTIP)}
         disabled={disabled}
       />
-      <AutocompleteCombobox
-        ariaLabel="agent"
-        items={agents.map((agent) => ({ value: agent, label: agent }))}
-        defaultKey={settings.AGENT}
-        onChange={onAgentChange}
-        tooltip={t(I18nKey.SETTINGS$AGENT_TOOLTIP)}
-        disabled={disabled || !isAgentSelectEnabled}
-      />
-      <Switch
-        defaultSelected={false}
-        isSelected={isAgentSelectEnabled}
-        onValueChange={setIsAgentSelectEnabled}
-        aria-label="enableagentselect"
-        data-testid="enableagentselect"
-      >
-        {t(I18nKey.SETTINGS$AGENT_SELECT_ENABLED)}
-      </Switch>
-      <AutocompleteCombobox
-        ariaLabel="securityanalyzer"
-        items={securityAnalyzers.map((securityAnalyzer) => ({
-          value: securityAnalyzer,
-          label: securityAnalyzer,
-        }))}
-        defaultKey={settings.SECURITY_ANALYZER}
-        onChange={onSecurityAnalyzerChange}
-        tooltip={t(I18nKey.SETTINGS$SECURITY_ANALYZER)}
-        disabled={disabled}
-      />
-      <Switch
-        aria-label="confirmationmode"
-        data-testid="confirmationmode"
-        defaultSelected={
-          settings.CONFIRMATION_MODE || !!settings.SECURITY_ANALYZER
-        }
-        onValueChange={onConfirmationModeChange}
-        isDisabled={disabled || !!settings.SECURITY_ANALYZER}
-        isSelected={settings.CONFIRMATION_MODE}
-      >
-        <Tooltip
-          content={t(I18nKey.SETTINGS$CONFIRMATION_MODE_TOOLTIP)}
-          closeDelay={100}
-          delay={500}
+      {enableAdvanced && (
+        <AutocompleteCombobox
+          ariaLabel="agent"
+          items={agents.map((agent) => ({ value: agent, label: agent }))}
+          defaultKey={settings.AGENT}
+          onChange={onAgentChange}
+          tooltip={t(I18nKey.SETTINGS$AGENT_TOOLTIP)}
+        />
+      )}
+      {enableAdvanced && (
+        <AutocompleteCombobox
+          ariaLabel="securityanalyzer"
+          items={securityAnalyzers.map((securityAnalyzer) => ({
+            value: securityAnalyzer,
+            label: securityAnalyzer,
+          }))}
+          defaultKey={settings.SECURITY_ANALYZER}
+          onChange={onSecurityAnalyzerChange}
+          tooltip={t(I18nKey.SETTINGS$SECURITY_ANALYZER)}
+          disabled={disabled}
+        />
+      )}
+      {enableAdvanced && (
+        <Switch
+          aria-label="confirmationmode"
+          data-testid="confirmationmode"
+          defaultSelected={
+            settings.CONFIRMATION_MODE || !!settings.SECURITY_ANALYZER
+          }
+          onValueChange={onConfirmationModeChange}
+          isDisabled={disabled || !!settings.SECURITY_ANALYZER}
+          isSelected={settings.CONFIRMATION_MODE}
         >
-          {t(I18nKey.SETTINGS$CONFIRMATION_MODE)}
-        </Tooltip>
-      </Switch>
+          <Tooltip
+            content={t(I18nKey.SETTINGS$CONFIRMATION_MODE_TOOLTIP)}
+            closeDelay={100}
+            delay={500}
+          >
+            {t(I18nKey.SETTINGS$CONFIRMATION_MODE)}
+          </Tooltip>
+        </Switch>
+      )}
     </>
   );
 }

+ 26 - 50
frontend/src/components/modals/settings/SettingsModal.test.tsx

@@ -4,7 +4,6 @@ import i18next from "i18next";
 import React from "react";
 import { renderWithProviders } from "test-utils";
 import { Mock } from "vitest";
-import toast from "#/utils/toast";
 import {
   Settings,
   getSettings,
@@ -15,7 +14,6 @@ import Session from "#/services/session";
 import { fetchAgents, fetchModels } from "#/services/options";
 import SettingsModal from "./SettingsModal";
 
-const toastSpy = vi.spyOn(toast, "settingsChanged");
 const i18nSpy = vi.spyOn(i18next, "changeLanguage");
 const startNewSessionSpy = vi.spyOn(Session, "startNewSession");
 vi.spyOn(Session, "isConnected").mockImplementation(() => true);
@@ -24,18 +22,14 @@ vi.mock("#/services/settings", async (importOriginal) => ({
   ...(await importOriginal<typeof import("#/services/settings")>()),
   getSettings: vi.fn().mockReturnValue({
     LLM_MODEL: "gpt-4o",
-    CUSTOM_LLM_MODEL: "",
-    USING_CUSTOM_MODEL: false,
     AGENT: "CodeActAgent",
     LANGUAGE: "en",
     LLM_API_KEY: "sk-...",
-    CONFIRMATION_MODE: true,
-    SECURITY_ANALYZER: "invariant",
+    CONFIRMATION_MODE: false,
+    SECURITY_ANALYZER: "",
   }),
   getDefaultSettings: vi.fn().mockReturnValue({
     LLM_MODEL: "gpt-4o",
-    CUSTOM_LLM_MODEL: "",
-    USING_CUSTOM_MODEL: false,
     AGENT: "CodeActAgent",
     LANGUAGE: "en",
     LLM_API_KEY: "",
@@ -98,8 +92,7 @@ describe("SettingsModal", () => {
   it("should disabled the save button if the settings contain a missing value", async () => {
     const onOpenChangeMock = vi.fn();
     (getSettings as Mock).mockReturnValueOnce({
-      LLM_MODEL: "gpt-4o",
-      AGENT: "",
+      LLM_MODEL: "",
     });
     await act(async () =>
       renderWithProviders(
@@ -115,13 +108,11 @@ describe("SettingsModal", () => {
   describe("onHandleSave", () => {
     const initialSettings: Settings = {
       LLM_MODEL: "gpt-4o",
-      CUSTOM_LLM_MODEL: "",
-      USING_CUSTOM_MODEL: false,
       AGENT: "CodeActAgent",
       LANGUAGE: "en",
       LLM_API_KEY: "sk-...",
-      CONFIRMATION_MODE: true,
-      SECURITY_ANALYZER: "invariant",
+      SECURITY_ANALYZER: "",
+      CONFIRMATION_MODE: false,
     };
 
     it("should save the settings", async () => {
@@ -135,8 +126,10 @@ describe("SettingsModal", () => {
       await assertModelsAndAgentsFetched();
 
       const saveButton = screen.getByRole("button", { name: /save/i });
-      const providerInput = screen.getByRole("combobox", { name: "Provider" });
-      const modelInput = screen.getByRole("combobox", { name: "Model" });
+      const providerInput = screen.getByRole("combobox", {
+        name: "LLM Provider",
+      });
+      const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
 
       await user.click(providerInput);
       const azure = screen.getByText("Azure");
@@ -164,8 +157,10 @@ describe("SettingsModal", () => {
       );
 
       const saveButton = screen.getByRole("button", { name: /save/i });
-      const providerInput = screen.getByRole("combobox", { name: "Provider" });
-      const modelInput = screen.getByRole("combobox", { name: "Model" });
+      const providerInput = screen.getByRole("combobox", {
+        name: "LLM Provider",
+      });
+      const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
 
       await user.click(providerInput);
       const openai = screen.getByText("OpenAI");
@@ -180,32 +175,6 @@ describe("SettingsModal", () => {
       expect(startNewSessionSpy).toHaveBeenCalled();
     });
 
-    it("should display a toast for every change", async () => {
-      const user = userEvent.setup();
-      const onOpenChangeMock = vi.fn();
-      await act(async () =>
-        renderWithProviders(
-          <SettingsModal isOpen onOpenChange={onOpenChangeMock} />,
-        ),
-      );
-
-      const saveButton = screen.getByRole("button", { name: /save/i });
-      const providerInput = screen.getByRole("combobox", { name: "Provider" });
-      const modelInput = screen.getByRole("combobox", { name: "Model" });
-
-      await user.click(providerInput);
-      const cohere = screen.getByText("cohere");
-      await user.click(cohere);
-
-      await user.click(modelInput);
-      const model3 = screen.getByText("command-r-v1:0");
-      await user.click(model3);
-
-      await user.click(saveButton);
-
-      expect(toastSpy).toHaveBeenCalledTimes(4);
-    });
-
     it("should change the language", async () => {
       const user = userEvent.setup();
       const onOpenChangeMock = vi.fn();
@@ -230,6 +199,10 @@ describe("SettingsModal", () => {
     it("should close the modal", async () => {
       const user = userEvent.setup();
       const onOpenChangeMock = vi.fn();
+      (getSettings as Mock).mockReturnValueOnce({
+        LLM_MODEL: "gpt-4o",
+        LLM_API_KEY: "sk-...",
+      });
       await act(async () =>
         renderWithProviders(
           <SettingsModal isOpen onOpenChange={onOpenChangeMock} />,
@@ -241,8 +214,10 @@ describe("SettingsModal", () => {
       });
 
       const saveButton = screen.getByRole("button", { name: /save/i });
-      const providerInput = screen.getByRole("combobox", { name: "Provider" });
-      const modelInput = screen.getByRole("combobox", { name: "Model" });
+      const providerInput = screen.getByRole("combobox", {
+        name: "LLM Provider",
+      });
+      const modelInput = screen.getByRole("combobox", { name: "LLM Model" });
 
       await user.click(providerInput);
       const cohere = screen.getByText("cohere");
@@ -252,6 +227,7 @@ describe("SettingsModal", () => {
       const model3 = screen.getByText("command-r-v1:0");
       await user.click(model3);
 
+      expect(saveButton).not.toBeDisabled();
       await user.click(saveButton);
 
       expect(onOpenChangeMock).toHaveBeenCalledWith(false);
@@ -261,16 +237,16 @@ describe("SettingsModal", () => {
   it("should reset settings to defaults when the 'reset to defaults' button is clicked", async () => {
     const user = userEvent.setup();
     const onOpenChangeMock = vi.fn();
+    (getSettings as Mock).mockReturnValueOnce({
+      LLM_MODEL: "gpt-4o",
+      SECURITY_ANALYZER: "fakeanalyzer",
+    });
     await act(async () =>
       renderWithProviders(
         <SettingsModal isOpen onOpenChange={onOpenChangeMock} />,
       ),
     );
 
-    // We need to enable the agent select first
-    const agentSwitch = screen.getByTestId("enableagentselect");
-    await user.click(agentSwitch);
-
     const resetButton = screen.getByRole("button", {
       name: /MODAL_RESET_BUTTON_LABEL/i,
     });

+ 2 - 30
frontend/src/components/modals/settings/SettingsModal.tsx

@@ -17,7 +17,6 @@ import {
   Settings,
   getSettings,
   getDefaultSettings,
-  getSettingsDifference,
   settingsAreUpToDate,
   maybeMigrateSettings,
   saveSettings,
@@ -31,7 +30,7 @@ interface SettingsProps {
   onOpenChange: (isOpen: boolean) => void;
 }
 
-const REQUIRED_SETTINGS = ["LLM_MODEL", "AGENT"];
+const REQUIRED_SETTINGS = ["LLM_MODEL"];
 
 function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
   const { t } = useTranslation();
@@ -83,20 +82,6 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
     }));
   };
 
-  const handleCustomModelChange = (model: string) => {
-    setSettings((prev) => ({
-      ...prev,
-      CUSTOM_LLM_MODEL: model,
-    }));
-  };
-
-  const handleModelTypeChange = (type: "custom" | "default") => {
-    setSettings((prev) => ({
-      ...prev,
-      USING_CUSTOM_MODEL: type === "custom",
-    }));
-  };
-
   const handleAgentChange = (agent: string) => {
     setSettings((prev) => ({ ...prev, AGENT: agent }));
   };
@@ -131,28 +116,17 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
   };
 
   const handleSaveSettings = () => {
-    const updatedSettings = getSettingsDifference(settings);
     saveSettings(settings);
     i18next.changeLanguage(settings.LANGUAGE);
     Session.startNewSession();
 
-    const sensitiveKeys = ["LLM_API_KEY"];
-
-    Object.entries(updatedSettings).forEach(([key, value]) => {
-      if (!sensitiveKeys.includes(key)) {
-        toast.settingsChanged(`${key} set to "${value}"`);
-      } else {
-        toast.settingsChanged(`${key} has been updated securely.`);
-      }
-    });
-
     localStorage.setItem(
       `API_KEY_${settings.LLM_MODEL || models[0]}`,
       settings.LLM_API_KEY,
     );
   };
 
-  let subtitle = t(I18nKey.CONFIGURATION$MODAL_SUB_TITLE);
+  let subtitle = "";
   if (loading) {
     subtitle = t(I18nKey.CONFIGURATION$AGENT_LOADING);
   } else if (agentIsRunning) {
@@ -205,8 +179,6 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
           agents={agents}
           securityAnalyzers={securityAnalyzers}
           onModelChange={handleModelChange}
-          onCustomModelChange={handleCustomModelChange}
-          onModelTypeChange={handleModelTypeChange}
           onAgentChange={handleAgentChange}
           onLanguageChange={handleLanguageChange}
           onAPIKeyChange={handleAPIKeyChange}

+ 0 - 12
frontend/src/i18n/translation.json

@@ -250,18 +250,6 @@
     "fr": "Configuration",
     "tr": "Konfigürasyon"
   },
-  "CONFIGURATION$MODAL_SUB_TITLE": {
-    "en": "Adjust settings to your liking",
-    "zh-CN": "根据您的喜好调整设置",
-    "de": "Passen Sie die Einstellungen nach Ihren Wünschen an ",
-    "ko-KR": "원하는 대로 설정 조정",
-    "no": "Juster innstillinger etter dine ønsker ",
-    "zh-TW": "調整設定以符合您的喜好",
-    "it": "Regola le impostazioni in base alle tue preferenze",
-    "pt": "Ajuste as configurações de acordo com sua preferência",
-    "es": "Ajusta la configuración a tu gusto",
-    "tr": "Ayarları isteğinize göre ayarlayın"
-  },
   "CONFIGURATION$MODEL_SELECT_LABEL": {
     "en": "Model",
     "zh-CN": "模型",

+ 0 - 31
frontend/src/services/session.test.ts

@@ -19,8 +19,6 @@ describe("startNewSession", () => {
   it("Should start a new session with the current settings", () => {
     const settings: Settings = {
       LLM_MODEL: "llm_value",
-      CUSTOM_LLM_MODEL: "",
-      USING_CUSTOM_MODEL: false,
       AGENT: "agent_value",
       LANGUAGE: "language_value",
       LLM_API_KEY: "sk-...",
@@ -39,33 +37,4 @@ describe("startNewSession", () => {
     expect(setupSpy).toHaveBeenCalledTimes(1);
     expect(sendSpy).toHaveBeenCalledWith(JSON.stringify(event));
   });
-
-  it("should start with the custom llm if set", () => {
-    const settings: Settings = {
-      LLM_MODEL: "llm_value",
-      CUSTOM_LLM_MODEL: "custom_llm_value",
-      USING_CUSTOM_MODEL: true,
-      AGENT: "agent_value",
-      LANGUAGE: "language_value",
-      LLM_API_KEY: "sk-...",
-      CONFIRMATION_MODE: true,
-      SECURITY_ANALYZER: "analyzer",
-    };
-
-    const event = {
-      action: ActionType.INIT,
-      args: settings,
-    };
-
-    saveSettings(settings);
-    Session.startNewSession();
-
-    expect(setupSpy).toHaveBeenCalledTimes(1);
-    expect(sendSpy).toHaveBeenCalledWith(
-      JSON.stringify({
-        ...event,
-        args: { ...settings, LLM_MODEL: "custom_llm_value" },
-      }),
-    );
-  });
 });

+ 0 - 3
frontend/src/services/session.ts

@@ -50,9 +50,6 @@ class Session {
       action: ActionType.INIT,
       args: {
         ...settings,
-        LLM_MODEL: settings.USING_CUSTOM_MODEL
-          ? settings.CUSTOM_LLM_MODEL
-          : settings.LLM_MODEL,
       },
     };
     const eventString = JSON.stringify(event);

+ 1 - 62
frontend/src/services/settings.test.ts

@@ -3,7 +3,6 @@ import {
   DEFAULT_SETTINGS,
   Settings,
   getSettings,
-  getSettingsDifference,
   saveSettings,
 } from "./settings";
 
@@ -18,8 +17,6 @@ describe("getSettings", () => {
   it("should get the stored settings", () => {
     (localStorage.getItem as Mock)
       .mockReturnValueOnce("llm_value")
-      .mockReturnValueOnce("custom_llm_value")
-      .mockReturnValueOnce("true")
       .mockReturnValueOnce("agent_value")
       .mockReturnValueOnce("language_value")
       .mockReturnValueOnce("api_key")
@@ -30,8 +27,6 @@ describe("getSettings", () => {
 
     expect(settings).toEqual({
       LLM_MODEL: "llm_value",
-      CUSTOM_LLM_MODEL: "custom_llm_value",
-      USING_CUSTOM_MODEL: true,
       AGENT: "agent_value",
       LANGUAGE: "language_value",
       LLM_API_KEY: "api_key",
@@ -55,8 +50,6 @@ describe("getSettings", () => {
 
     expect(settings).toEqual({
       LLM_MODEL: DEFAULT_SETTINGS.LLM_MODEL,
-      CUSTOM_LLM_MODEL: "",
-      USING_CUSTOM_MODEL: DEFAULT_SETTINGS.USING_CUSTOM_MODEL,
       AGENT: DEFAULT_SETTINGS.AGENT,
       LANGUAGE: DEFAULT_SETTINGS.LANGUAGE,
       LLM_API_KEY: "",
@@ -70,8 +63,6 @@ describe("saveSettings", () => {
   it("should save the settings", () => {
     const settings: Settings = {
       LLM_MODEL: "llm_value",
-      CUSTOM_LLM_MODEL: "custom_llm_value",
-      USING_CUSTOM_MODEL: true,
       AGENT: "agent_value",
       LANGUAGE: "language_value",
       LLM_API_KEY: "some_key",
@@ -82,14 +73,6 @@ describe("saveSettings", () => {
     saveSettings(settings);
 
     expect(localStorage.setItem).toHaveBeenCalledWith("LLM_MODEL", "llm_value");
-    expect(localStorage.setItem).toHaveBeenCalledWith(
-      "CUSTOM_LLM_MODEL",
-      "custom_llm_value",
-    );
-    expect(localStorage.setItem).toHaveBeenCalledWith(
-      "USING_CUSTOM_MODEL",
-      "true",
-    );
     expect(localStorage.setItem).toHaveBeenCalledWith("AGENT", "agent_value");
     expect(localStorage.setItem).toHaveBeenCalledWith(
       "LANGUAGE",
@@ -110,7 +93,7 @@ describe("saveSettings", () => {
 
     expect(localStorage.setItem).toHaveBeenCalledTimes(2);
     expect(localStorage.setItem).toHaveBeenCalledWith("LLM_MODEL", "llm_value");
-    expect(localStorage.setItem).toHaveBeenCalledWith("SETTINGS_VERSION", "1");
+    expect(localStorage.setItem).toHaveBeenCalledWith("SETTINGS_VERSION", "2");
   });
 
   it("should not save invalid settings", () => {
@@ -135,47 +118,3 @@ describe("saveSettings", () => {
     );
   });
 });
-
-describe("getSettingsDifference", () => {
-  beforeEach(() => {
-    (localStorage.getItem as Mock)
-      .mockReturnValueOnce("llm_value")
-      .mockReturnValueOnce("custom_llm_value")
-      .mockReturnValueOnce("false")
-      .mockReturnValueOnce("agent_value")
-      .mockReturnValueOnce("language_value");
-  });
-
-  it("should return updated settings", () => {
-    const settings = {
-      LLM_MODEL: "new_llm_value",
-      CUSTOM_LLM_MODEL: "custom_llm_value",
-      USING_CUSTOM_MODEL: true,
-      AGENT: "new_agent_value",
-      LANGUAGE: "language_value",
-    };
-
-    const updatedSettings = getSettingsDifference(settings);
-
-    expect(updatedSettings).toEqual({
-      USING_CUSTOM_MODEL: true,
-      LLM_MODEL: "new_llm_value",
-      AGENT: "new_agent_value",
-    });
-  });
-
-  it("should not handle invalid settings", () => {
-    const settings = {
-      LLM_MODEL: "new_llm_value",
-      AGENT: "new_agent_value",
-      INVALID: "invalid_value",
-    };
-
-    const updatedSettings = getSettingsDifference(settings);
-
-    expect(updatedSettings).toEqual({
-      LLM_MODEL: "new_llm_value",
-      AGENT: "new_agent_value",
-    });
-  });
-});

+ 9 - 43
frontend/src/services/settings.ts

@@ -1,9 +1,7 @@
-const LATEST_SETTINGS_VERSION = 1;
+const LATEST_SETTINGS_VERSION = 2;
 
 export type Settings = {
   LLM_MODEL: string;
-  CUSTOM_LLM_MODEL: string;
-  USING_CUSTOM_MODEL: boolean;
   AGENT: string;
   LANGUAGE: string;
   LLM_API_KEY: string;
@@ -11,12 +9,8 @@ export type Settings = {
   SECURITY_ANALYZER: string;
 };
 
-type SettingsInput = Settings[keyof Settings];
-
 export const DEFAULT_SETTINGS: Settings = {
   LLM_MODEL: "openai/gpt-4o",
-  CUSTOM_LLM_MODEL: "",
-  USING_CUSTOM_MODEL: false,
   AGENT: "CodeActAgent",
   LANGUAGE: "en",
   LLM_API_KEY: "",
@@ -46,6 +40,14 @@ export const maybeMigrateSettings = () => {
   if (currentVersion < 1) {
     localStorage.setItem("AGENT", DEFAULT_SETTINGS.AGENT);
   }
+  if (currentVersion < 2) {
+    const customModel = localStorage.getItem("CUSTOM_LLM_MODEL");
+    if (customModel) {
+      localStorage.setItem("LLM_MODEL", customModel);
+    }
+    localStorage.removeItem("CUSTOM_LLM_MODEL");
+    localStorage.removeItem("USING_CUSTOM_MODEL");
+  }
 };
 
 /**
@@ -58,9 +60,6 @@ export const getDefaultSettings = (): Settings => DEFAULT_SETTINGS;
  */
 export const getSettings = (): Settings => {
   const model = localStorage.getItem("LLM_MODEL");
-  const customModel = localStorage.getItem("CUSTOM_LLM_MODEL");
-  const usingCustomModel =
-    localStorage.getItem("USING_CUSTOM_MODEL") === "true";
   const agent = localStorage.getItem("AGENT");
   const language = localStorage.getItem("LANGUAGE");
   const apiKey = localStorage.getItem("LLM_API_KEY");
@@ -69,8 +68,6 @@ export const getSettings = (): Settings => {
 
   return {
     LLM_MODEL: model || DEFAULT_SETTINGS.LLM_MODEL,
-    CUSTOM_LLM_MODEL: customModel || DEFAULT_SETTINGS.CUSTOM_LLM_MODEL,
-    USING_CUSTOM_MODEL: usingCustomModel || DEFAULT_SETTINGS.USING_CUSTOM_MODEL,
     AGENT: agent || DEFAULT_SETTINGS.AGENT,
     LANGUAGE: language || DEFAULT_SETTINGS.LANGUAGE,
     LLM_API_KEY: apiKey || DEFAULT_SETTINGS.LLM_API_KEY,
@@ -93,34 +90,3 @@ export const saveSettings = (settings: Partial<Settings>) => {
   });
   localStorage.setItem("SETTINGS_VERSION", LATEST_SETTINGS_VERSION.toString());
 };
-
-/**
- * Get the difference between the current settings and the provided settings.
- * Useful for notifying the user of exact changes.
- *
- * @example
- * // Assuming the current settings are: { LLM_MODEL: "gpt-4o", AGENT: "CodeActAgent", LANGUAGE: "en" }
- * const updatedSettings = getSettingsDifference({ LLM_MODEL: "gpt-4o", AGENT: "OTHER_AGENT", LANGUAGE: "en" });
- * // updatedSettings = { AGENT: "OTHER_AGENT" }
- *
- * @param settings - the settings to compare
- * @returns the updated settings
- */
-export const getSettingsDifference = (settings: Partial<Settings>) => {
-  const currentSettings = getSettings();
-  const updatedSettings: Partial<Settings> = {};
-
-  Object.keys(settings).forEach((key) => {
-    const typedKey = key as keyof Settings;
-    if (
-      validKeys.includes(typedKey) &&
-      settings[typedKey] !== currentSettings[typedKey]
-    ) {
-      (updatedSettings[typedKey] as SettingsInput) = settings[
-        typedKey
-      ] as SettingsInput;
-    }
-  });
-
-  return updatedSettings;
-};