Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set API key from settings modal #1319

Merged
merged 16 commits into from Apr 29, 2024
19 changes: 19 additions & 0 deletions frontend/src/components/modals/settings/SettingsForm.test.tsx
Expand Up @@ -8,16 +8,19 @@ import SettingsForm from "./SettingsForm";
const onModelChangeMock = vi.fn();
const onAgentChangeMock = vi.fn();
const onLanguageChangeMock = vi.fn();
const onAPIKeyChangeMock = vi.fn();

const renderSettingsForm = (settings: Partial<Settings>) => {
renderWithProviders(
<SettingsForm
settings={settings}
models={["model1", "model2", "model3"]}
agents={["agent1", "agent2", "agent3"]}
apiKey="sk-..."
onModelChange={onModelChangeMock}
onAgentChange={onAgentChangeMock}
onLanguageChange={onLanguageChangeMock}
onAPIKeyChange={onAPIKeyChangeMock}
/>,
);
};
Expand All @@ -29,10 +32,12 @@ describe("SettingsForm", () => {
const modelInput = screen.getByRole("combobox", { name: "model" });
const agentInput = screen.getByRole("combobox", { name: "agent" });
const languageInput = screen.getByRole("combobox", { name: "language" });
const apiKeyInput = screen.getByTestId("apikey");

expect(modelInput).toHaveValue("model1");
expect(agentInput).toHaveValue("agent1");
expect(languageInput).toHaveValue("English");
expect(apiKeyInput).toHaveValue("sk-...");
});

it("should display the existing values if it they are present", () => {
Expand All @@ -57,9 +62,11 @@ describe("SettingsForm", () => {
settings={{}}
models={["model1", "model2", "model3"]}
agents={["agent1", "agent2", "agent3"]}
apiKey="sk-..."
onModelChange={onModelChangeMock}
onAgentChange={onAgentChangeMock}
onLanguageChange={onLanguageChangeMock}
onAPIKeyChange={onAPIKeyChangeMock}
/>,
{ preloadedState: { agent: { curTaskState: AgentTaskState.RUNNING } } },
);
Expand Down Expand Up @@ -87,6 +94,7 @@ describe("SettingsForm", () => {
});

expect(onModelChangeMock).toHaveBeenCalledWith("model3");
expect(onAPIKeyChangeMock).toHaveBeenCalledWith("");
Sparkier marked this conversation as resolved.
Show resolved Hide resolved
});

it("should call the onAgentChange handler when the agent changes", () => {
Expand Down Expand Up @@ -120,5 +128,16 @@ describe("SettingsForm", () => {

expect(onLanguageChangeMock).toHaveBeenCalledWith("Français");
});

it("should call the onAPIKeyChange handler when the API key changes", () => {
renderSettingsForm({});

const apiKeyInput = screen.getByTestId("apikey");
act(() => {
userEvent.type(apiKeyInput, "x");
});

expect(onAPIKeyChangeMock).toHaveBeenCalledWith("sk-...x");
});
});
});
44 changes: 43 additions & 1 deletion frontend/src/components/modals/settings/SettingsForm.tsx
@@ -1,5 +1,7 @@
import { Input, useDisclosure } from "@nextui-org/react";
import React, { useEffect } from "react";
import { useTranslation } from "react-i18next";
import { FaEye, FaEyeSlash } from "react-icons/fa";
import { useSelector } from "react-redux";
import { AvailableLanguages } from "../../../i18n";
import { I18nKey } from "../../../i18n/declaration";
Expand All @@ -10,24 +12,29 @@ import { AutocompleteCombobox } from "./AutocompleteCombobox";
interface SettingsFormProps {
settings: Partial<Settings>;
models: string[];
apiKey: string;
agents: string[];

onModelChange: (model: string) => void;
onAPIKeyChange: (apiKey: string) => void;
onAgentChange: (agent: string) => void;
onLanguageChange: (language: string) => void;
}

function SettingsForm({
settings,
models,
apiKey,
agents,
onModelChange,
onAPIKeyChange,
onAgentChange,
onLanguageChange,
}: SettingsFormProps) {
const { t } = useTranslation();
const { curTaskState } = useSelector((state: RootState) => state.agent);
const [disabled, setDisabled] = React.useState<boolean>(false);
const { isOpen: isVisible, onOpenChange: onVisibleChange } = useDisclosure();

useEffect(() => {
if (
Expand All @@ -46,11 +53,46 @@ function SettingsForm({
ariaLabel="model"
items={models.map((model) => ({ value: model, label: model }))}
defaultKey={settings.LLM_MODEL || models[0]}
onChange={onModelChange}
onChange={(e) => {
const key = localStorage.getItem(
`API_KEY_${settings.LLM_MODEL || models[0]}`,
);
onAPIKeyChange(key || "");
onModelChange(e);
}}
tooltip={t(I18nKey.SETTINGS$MODEL_TOOLTIP)}
allowCustomValue // user can type in a custom LLM model that is not in the list
disabled={disabled}
/>
<Input
label="API Key"
disabled={disabled}
aria-label="apikey"
data-testid="apikey"
placeholder={t(I18nKey.SETTINGS$API_KEY_PLACEHOLDER)}
type={isVisible ? "text" : "password"}
value={apiKey}
onChange={(e) => {
Sparkier marked this conversation as resolved.
Show resolved Hide resolved
localStorage.setItem(
`API_KEY_${settings.LLM_MODEL || models[0]}`,
e.target.value,
);
onAPIKeyChange(e.target.value);
}}
endContent={
<button
className="focus:outline-none"
type="button"
onClick={onVisibleChange}
>
{isVisible ? (
<FaEye className="text-2xl text-default-400 pointer-events-none" />
) : (
<FaEyeSlash className="text-2xl text-default-400 pointer-events-none" />
)}
</button>
}
/>
<AutocompleteCombobox
ariaLabel="agent"
items={agents.map((agent) => ({ value: agent, label: agent }))}
Expand Down
20 changes: 13 additions & 7 deletions frontend/src/components/modals/settings/SettingsModal.tsx
@@ -1,16 +1,16 @@
import React from "react";
import { useTranslation } from "react-i18next";
import { Spinner } from "@nextui-org/react";
import BaseModal from "../base-modal/BaseModal";
import SettingsForm from "./SettingsForm";
import { AvailableLanguages } from "#/i18n";
import { I18nKey } from "#/i18n/declaration";
import {
fetchAgents,
fetchModels,
getCurrentSettings,
saveSettings,
} from "#/services/settingsService";
import { I18nKey } from "#/i18n/declaration";
import { AvailableLanguages } from "#/i18n";
import { Spinner } from "@nextui-org/react";
import React from "react";
import { useTranslation } from "react-i18next";
import BaseModal from "../base-modal/BaseModal";
import SettingsForm from "./SettingsForm";

interface SettingsProps {
isOpen: boolean;
Expand All @@ -25,6 +25,9 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
const [agents, setAgents] = React.useState<string[]>([]);
const [settings, setSettings] =
React.useState<Partial<Settings>>(currentSettings);
const [apiKey, setApiKey] = React.useState<string>(
localStorage.getItem(`API_KEY_${settings.LLM_MODEL}`) || "",
Copy link
Collaborator

@amanape amanape Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've extended getCurrentSettings to also include the API key. Maybe you can utilize the settings state that now already stores this data directly rather than use a separate state to handle the API key.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good point.

);

const [loading, setLoading] = React.useState(true);

Expand Down Expand Up @@ -67,6 +70,7 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
{
label: t(I18nKey.CONFIGURATION$MODAL_SAVE_BUTTON_LABEL),
action: () => {
setSettings((prev) => ({ ...prev, LLM_API_KEY: apiKey }));
saveSettings(settings);
},
closeAfterAction: true,
Expand All @@ -87,10 +91,12 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
<SettingsForm
settings={settings}
models={models}
apiKey={apiKey}
agents={agents}
onModelChange={handleModelChange}
onAgentChange={handleAgentChange}
onLanguageChange={handleLanguageChange}
onAPIKeyChange={(key) => setApiKey(key)}
/>
)}
</BaseModal>
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/i18n/translation.json
Expand Up @@ -324,5 +324,9 @@
"SETTINGS$DISABLED_RUNNING": {
"en": "Cannot be changed while the agent is running.",
"de": "Kann nicht geändert werden während ein Task ausgeführt wird."
},
"SETTINGS$API_KEY_PLACEHOLDER": {
"en": "Enter your API key.",
"de": "Model API key."
}
}
1 change: 1 addition & 0 deletions frontend/src/services/settings.d.ts
@@ -1,5 +1,6 @@
type Settings = {
LLM_MODEL: string;
LLM_API_KEY: string;
AGENT: string;
LANGUAGE: string;
};
6 changes: 5 additions & 1 deletion frontend/src/services/settingsService.ts
@@ -1,8 +1,8 @@
import { setByKey } from "#/state/settingsSlice";
import { setInitialized } from "#/state/taskSlice";
import store from "#/store";
import ActionType from "#/types/ActionType";
import { SupportedSettings } from "#/types/ConfigType";
import { setByKey } from "#/state/settingsSlice";
import toast from "#/utils/toast";
import Socket from "./socket";

Expand Down Expand Up @@ -30,6 +30,7 @@ const DEFAULT_SETTINGS: Settings = {
LLM_MODEL: "gpt-3.5-turbo",
AGENT: "MonologueAgent",
LANGUAGE: "en",
LLM_API_KEY: "",
};

export const getSettingOrDefault = (key: string): string => {
Expand All @@ -41,6 +42,9 @@ export const getCurrentSettings = (): Settings => ({
LLM_MODEL: getSettingOrDefault("LLM_MODEL"),
AGENT: getSettingOrDefault("AGENT"),
LANGUAGE: getSettingOrDefault("LANGUAGE"),
LLM_API_KEY:
localStorage.getItem(`API_KEY_${getSettingOrDefault("LLM_MODEL")}`) ||
DEFAULT_SETTINGS.LLM_API_KEY,
});

// Function to merge and update settings
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/types/ConfigType.tsx
Expand Up @@ -2,12 +2,14 @@ enum ArgConfigType {
LLM_MODEL = "LLM_MODEL",
AGENT = "AGENT",
LANGUAGE = "LANGUAGE",
LLM_API_KEY = "LLM_API_KEY",
}

const SupportedSettings: string[] = [
ArgConfigType.LLM_MODEL,
ArgConfigType.AGENT,
ArgConfigType.LANGUAGE,
ArgConfigType.LLM_API_KEY,
];

export { ArgConfigType, SupportedSettings };
4 changes: 2 additions & 2 deletions opendevin/server/agent/agent.py
@@ -1,5 +1,5 @@
import asyncio
from typing import Optional, Dict, List
from typing import Dict, List, Optional

from opendevin import config
from opendevin.action import (
Expand Down Expand Up @@ -136,7 +136,7 @@ async def create_controller(self, start_event: dict):
} # remove empty values, prevent FE from sending empty strings
agent_cls = self.get_arg_or_default(args, ConfigType.AGENT)
model = self.get_arg_or_default(args, ConfigType.LLM_MODEL)
api_key = config.get(ConfigType.LLM_API_KEY)
api_key = self.get_arg_or_default(args, ConfigType.LLM_API_KEY)
api_base = config.get(ConfigType.LLM_BASE_URL)
container_image = config.get(ConfigType.SANDBOX_CONTAINER_IMAGE)
max_iterations = self.get_arg_or_default(args, ConfigType.MAX_ITERATIONS)
Expand Down