Skip to content

Commit

Permalink
fix(frontend): a few issues with settings (All-Hands-AI#5940)
Browse files Browse the repository at this point in the history
Co-authored-by: Robert Brennan <[email protected]>
Co-authored-by: Robert Brennan <[email protected]>
  • Loading branch information
3 people authored Dec 31, 2024
1 parent 3e9ba40 commit b6c8aa2
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 38 deletions.
4 changes: 1 addition & 3 deletions frontend/src/api/open-hands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
Conversation,
} from "./open-hands.types";
import { openHands } from "./open-hands-axios";
import { ApiSettings, Settings } from "#/services/settings";
import { ApiSettings } from "#/services/settings";

class OpenHands {
/**
Expand Down Expand Up @@ -238,13 +238,11 @@ class OpenHands {
}

static async createConversation(
settings: Settings,
githubToken?: string,
selectedRepository?: string,
): Promise<Conversation> {
const body = {
github_token: githubToken,
args: settings,
selected_repository: selectedRepository,
};

Expand Down
3 changes: 0 additions & 3 deletions frontend/src/hooks/mutation/use-create-conversation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ import OpenHands from "#/api/open-hands";
import { setInitialQuery } from "#/state/initial-query-slice";
import { RootState } from "#/store";
import { useAuth } from "#/context/auth-context";
import { useSettings } from "../query/use-settings";

export const useCreateConversation = () => {
const navigate = useNavigate();
const dispatch = useDispatch();
const { gitHubToken } = useAuth();
const { data: settings } = useSettings();
const queryClient = useQueryClient();

const { selectedRepository, files } = useSelector(
Expand All @@ -27,7 +25,6 @@ export const useCreateConversation = () => {

if (variables.q) dispatch(setInitialQuery(variables.q));
return OpenHands.createConversation(
settings,
gitHubToken || undefined,
selectedRepository || undefined,
);
Expand Down
7 changes: 4 additions & 3 deletions frontend/src/hooks/mutation/use-save-settings.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { useMutation, useQueryClient } from "@tanstack/react-query";
import {
ApiSettings,
DEFAULT_SETTINGS,
LATEST_SETTINGS_VERSION,
Settings,
} from "#/services/settings";
Expand All @@ -11,11 +12,11 @@ const saveSettingsMutationFn = async (settings: Partial<Settings>) => {
const apiSettings: Partial<ApiSettings> = {
llm_model: settings.LLM_MODEL,
llm_base_url: settings.LLM_BASE_URL,
agent: settings.AGENT,
language: settings.LANGUAGE,
agent: settings.AGENT || DEFAULT_SETTINGS.AGENT,
language: settings.LANGUAGE || DEFAULT_SETTINGS.LANGUAGE,
confirmation_mode: settings.CONFIRMATION_MODE,
security_analyzer: settings.SECURITY_ANALYZER,
llm_api_key: settings.LLM_API_KEY,
llm_api_key: settings.LLM_API_KEY?.trim() || undefined,
};

await OpenHands.saveSettings(apiSettings);
Expand Down
37 changes: 24 additions & 13 deletions frontend/src/hooks/query/use-settings.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
import { useQuery } from "@tanstack/react-query";
import React from "react";
import posthog from "posthog-js";
import { AxiosError } from "axios";
import { DEFAULT_SETTINGS, getLocalStorageSettings } from "#/services/settings";
import OpenHands from "#/api/open-hands";

const getSettingsQueryFn = async () => {
const apiSettings = await OpenHands.getSettings();
try {
const apiSettings = await OpenHands.getSettings();

if (apiSettings !== null) {
return {
LLM_MODEL: apiSettings.llm_model,
LLM_BASE_URL: apiSettings.llm_base_url,
AGENT: apiSettings.agent,
LANGUAGE: apiSettings.language,
CONFIRMATION_MODE: apiSettings.confirmation_mode,
SECURITY_ANALYZER: apiSettings.security_analyzer,
LLM_API_KEY: apiSettings.llm_api_key,
};
}
if (apiSettings !== null) {
return {
LLM_MODEL: apiSettings.llm_model,
LLM_BASE_URL: apiSettings.llm_base_url,
AGENT: apiSettings.agent,
LANGUAGE: apiSettings.language,
CONFIRMATION_MODE: apiSettings.confirmation_mode,
SECURITY_ANALYZER: apiSettings.security_analyzer,
LLM_API_KEY: apiSettings.llm_api_key,
};
}

return getLocalStorageSettings();
return getLocalStorageSettings();
} catch (error) {
if (error instanceof AxiosError) {
if (error.response?.status === 404) {
return DEFAULT_SETTINGS;
}
}

throw error;
}
};

export const useSettings = () => {
Expand Down
5 changes: 1 addition & 4 deletions openhands/server/routes/new_conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
from pydantic import BaseModel

from openhands.core.logger import openhands_logger as logger
from openhands.server.data_models.conversation_metadata import ConversationMetadata
from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl
from openhands.server.session.conversation_init_data import ConversationInitData
from openhands.server.shared import config, session_manager
from openhands.server.data_models.conversation_metadata import ConversationMetadata
from openhands.utils.async_utils import call_sync_from_async

app = APIRouter(prefix='/api')
Expand Down Expand Up @@ -38,9 +38,6 @@ async def new_conversation(request: Request, data: InitSessionRequest):
session_init_args: dict = {}
if settings:
session_init_args = {**settings.__dict__, **session_init_args}
if data.args:
for key, value in data.args.items():
session_init_args[key.lower()] = value

session_init_args['github_token'] = github_token
session_init_args['selected_repository'] = data.selected_repository
Expand Down
18 changes: 11 additions & 7 deletions openhands/server/routes/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@ async def load_settings(
try:
settings_store = await SettingsStoreImpl.get_instance(config, github_token)
settings = await settings_store.load()
if settings:
# For security reasons we don't ever send the api key to the client
settings.llm_api_key = 'SET' if settings.llm_api_key else None
if not settings:
return JSONResponse(
status_code=status.HTTP_404_NOT_FOUND,
content={'error': 'Settings not found'},
)

# For security reasons we don't ever send the api key to the client
settings.llm_api_key = 'SET' if settings.llm_api_key else None
return settings
except Exception as e:
logger.warning(f'Invalid token: {e}')
Expand All @@ -50,14 +55,13 @@ async def store_settings(
try:
settings_store = await SettingsStoreImpl.get_instance(config, github_token)
existing_settings = await settings_store.load()

if existing_settings:
# Only update settings that are not None with the new values
for key, value in settings.__dict__.items():
if value is None:
setattr(settings, key, getattr(existing_settings, key))
# LLM key isn't on the frontend, so we need to keep it if unset
if settings.llm_api_key is None:
settings.llm_api_key = existing_settings.llm_api_key
await settings_store.store(settings)

return JSONResponse(
status_code=status.HTTP_200_OK,
content={'message': 'Settings stored'},
Expand Down
8 changes: 3 additions & 5 deletions openhands/server/session/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,9 @@ async def initialize_agent(
# override default LLM config

default_llm_config = self.config.get_llm_config()
default_llm_config.model = settings.llm_model or default_llm_config.model
default_llm_config.api_key = settings.llm_api_key or default_llm_config.api_key
default_llm_config.base_url = (
settings.llm_base_url or default_llm_config.base_url
)
default_llm_config.model = settings.llm_model or ''
default_llm_config.api_key = settings.llm_api_key
default_llm_config.base_url = settings.llm_base_url

# TODO: override other LLM config & agent config groups (#2075)

Expand Down

0 comments on commit b6c8aa2

Please sign in to comment.