Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { ModelSelector } from "#/components/shared/modals/settings/model-selector";
import { server } from "#/mocks/node";

describe("ModelSelector — OpenHands round-trip", () => {
describe("ModelSelector — OpenHands provider", () => {
let providersCount = 0;
let verifiedCount = 0;
let modelsCount = 0;
Expand All @@ -20,7 +20,9 @@ describe("ModelSelector — OpenHands round-trip", () => {
server.use(
http.get("*/api/llm/providers", () => {
providersCount += 1;
return HttpResponse.json({ providers: ["anthropic", "openai"] });
return HttpResponse.json({
providers: ["openhands", "anthropic", "openai"],
});
}),
http.get("*/api/llm/models/verified", () => {
verifiedCount += 1;
Expand All @@ -47,25 +49,13 @@ describe("ModelSelector — OpenHands round-trip", () => {
);
}

it("shows OpenHands as the provider for a persisted litellm_proxy/<m> + All-Hands proxy base URL, fetching each LLM endpoint exactly once", async () => {
// Arrange — mirrors the post-save state: the SDK rewrote openhands/<m>
// to litellm_proxy/<m> on disk and pinned the All-Hands proxy base URL.
renderWithQuery(
<ModelSelector
currentModel="litellm_proxy/claude-opus-4-7"
currentBaseUrl="https://llm-proxy.app.all-hands.dev/"
/>,
);
it("shows OpenHands as the provider for a public openhands/<m> model", async () => {
Comment thread
enyst marked this conversation as resolved.
renderWithQuery(<ModelSelector currentModel="openhands/claude-opus-4-7" />);

// Act / Assert — the bootstrap effect must wait for the openhands
// verified list to load before resolving the displayed provider.
await waitFor(() => {
expect(screen.getByLabelText("LLM$PROVIDER")).toHaveValue("OpenHands");
});

// Assert — the three queries that all need verified-models data share
// a single cache entry, and the bootstrap effect picks the right
// provider on its first run, so /models is not fetched twice.
expect(providersCount).toBe(1);
expect(verifiedCount).toBe(1);
expect(modelsCount).toBe(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,10 @@ describe("LlmSettingsLocalView", () => {
});

describe("Basic tab save", () => {
it("drops hidden base_url values for OpenHands models", async () => {
it("drops stale base_url for OpenHands models in Basic mode", async () => {
// Arrange — a profile whose stored config pairs an OpenHands model with a
// stale base_url. In Basic mode the public provider prefix is enough; the
// SDK derives provider transport details when making LLM calls.
const user = userEvent.setup();
vi.mocked(ProfilesService.getProfile).mockResolvedValue({
name: "gpt-4-profile",
Expand All @@ -608,6 +611,7 @@ describe("LlmSettingsLocalView", () => {

renderWithProviders(<LlmSettingsLocalView />);

// Act — open the profile in edit mode, force the Basic tab, and save.
await user.click(screen.getAllByTestId("profile-menu-trigger")[0]);
await user.click(screen.getByTestId("profile-edit"));
await waitFor(() => {
Expand All @@ -621,11 +625,54 @@ describe("LlmSettingsLocalView", () => {
});
await user.click(screen.getByTestId("save-profile-btn"));

// Assert — the saved LLM config keeps the OpenHands model and drops the
// stale base_url instead of re-stamping a LiteLLM proxy detail.
await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled());
const savedLlm = mockSaveMutateAsync.mock.calls[0][0].request.llm;
expect(savedLlm.model).toBe("openhands/claude-opus-4-5-20251101");
expect(savedLlm).not.toHaveProperty("base_url");
});

it("drops base_url for stored litellm_proxy profiles in Basic mode", async () => {
// Arrange — legacy profiles may still contain a LiteLLM proxy model and
// proxy base_url. The SDK migration owns that compatibility; the Basic tab
// should not keep re-stamping transport details.
const user = userEvent.setup();
vi.mocked(ProfilesService.getProfile).mockResolvedValue({
name: "gpt-4-profile",
api_key_set: true,
config: {
model: "litellm_proxy/claude-opus-4-8",
api_key: "gAAAA_encrypted_key",
base_url: "https://llm-proxy.app.all-hands.dev/",
},
});
mockSaveMutateAsync.mockResolvedValueOnce({ success: true });

renderWithProviders(<LlmSettingsLocalView />);

// Act — open the profile in edit mode, force the Basic tab, and save
// without touching the model dropdown.
await user.click(screen.getAllByTestId("profile-menu-trigger")[0]);
await user.click(screen.getByTestId("profile-edit"));
await waitFor(() => {
expect(screen.getByTestId("profile-name-input")).toHaveValue(
"gpt-4-profile",
);
});
await user.click(await screen.findByTestId("sdk-section-basic-toggle"));
await waitFor(() => {
expect(screen.getByTestId("save-profile-btn")).not.toBeDisabled();
});
await user.click(screen.getByTestId("save-profile-btn"));

// Assert — the legacy model is preserved, but the Basic tab drops the
// base_url instead of reverse-mapping it in the frontend.
await waitFor(() => expect(mockSaveMutateAsync).toHaveBeenCalled());
const savedLlm = mockSaveMutateAsync.mock.calls[0][0].request.llm;
expect(savedLlm.model).toBe("litellm_proxy/claude-opus-4-8");
expect(savedLlm).not.toHaveProperty("base_url");
});
});

describe("All tab save", () => {
Expand Down
39 changes: 0 additions & 39 deletions __tests__/utils/normalize-display-model.test.ts

This file was deleted.

28 changes: 0 additions & 28 deletions __tests__/utils/openhands-llm.test.ts

This file was deleted.

22 changes: 4 additions & 18 deletions src/components/shared/modals/settings/model-selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@ import { HelpLink } from "#/ui/help-link";
import { PRODUCT_URL } from "#/utils/constants";
import { useSearchProviders } from "#/hooks/query/use-search-providers";
import { useProviderModels } from "#/hooks/query/use-provider-models";
import { useOpenhandsVerifiedModels } from "#/hooks/query/use-openhands-verified-models";
import { normalizeDisplayModel } from "#/utils/normalize-display-model";

interface ModelSelectorProps {
isDisabled?: boolean;
currentModel?: string;
currentBaseUrl?: string;
onChange?: (provider: string | null, model: string | null) => void;
onDefaultValuesChanged?: (
provider: string | null,
Expand All @@ -34,7 +31,6 @@ interface ModelSelectorProps {
export function ModelSelector({
isDisabled,
currentModel,
currentBaseUrl,
onChange,
onDefaultValuesChanged,
wrapperClassName,
Expand All @@ -47,7 +43,6 @@ export function ModelSelector({
const [selectedModel, setSelectedModel] = React.useState<string | null>(null);

const { data: providers = [] } = useSearchProviders();
const { data: openhandsVerifiedModels } = useOpenhandsVerifiedModels();
const {
data: providerModels = [],
isLoading: isLoadingModels,
Expand All @@ -73,24 +68,15 @@ export function ModelSelector({
);

React.useEffect(() => {
// Wait for the openhands verified list before initializing — otherwise a
// persisted `litellm_proxy/<m>` model would first land as `litellm_proxy`
// and only later flip to `openhands`, triggering a redundant /api/llm/models
// fetch for the throwaway provider value.
if (currentModel && openhandsVerifiedModels !== undefined) {
const displayModel = normalizeDisplayModel(
currentModel,
currentBaseUrl,
openhandsVerifiedModels,
);
const { provider, model } = extractModelAndProvider(displayModel);
if (currentModel) {
const { provider, model } = extractModelAndProvider(currentModel);
Comment thread
enyst marked this conversation as resolved.

setLitellmId(displayModel);
setLitellmId(currentModel);
setSelectedProvider(provider || null);
setSelectedModel(model);
onDefaultValuesChanged?.(provider || null, model);
}
}, [currentModel, currentBaseUrl, openhandsVerifiedModels]);
}, [currentModel]);

const handleChangeProvider = (provider: string) => {
setSelectedProvider(provider);
Expand Down
4 changes: 0 additions & 4 deletions src/components/shared/modals/settings/settings-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ export function SettingsForm({ settings, onClose }: SettingsFormProps) {

const isLLMKeySet = settings.llm_api_key_set;
const currentModel = getAgentSettingValue(settings, "llm.model");
const currentBaseUrl = getAgentSettingValue(settings, "llm.base_url");

return (
<div>
Expand All @@ -85,9 +84,6 @@ export function SettingsForm({ settings, onClose }: SettingsFormProps) {
currentModel={
typeof currentModel === "string" ? currentModel : undefined
}
currentBaseUrl={
typeof currentBaseUrl === "string" ? currentBaseUrl : undefined
}
wrapperClassName="!flex-col !gap-[17px]"
labelClassName={SETTINGS_FORM.LABEL_CLASSNAME}
/>
Expand Down
10 changes: 0 additions & 10 deletions src/routes/llm-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {
resolveLlmAuthType,
} from "#/constants/llm-subscription";
import { useOpenAISubscriptionModels } from "#/hooks/query/use-llm-subscription-models";
import { OPENHANDS_LLM_PROXY_BASE_URL } from "#/utils/openhands-llm";

const LLM_EXCLUDED_KEYS = new Set([
"llm.model",
Expand All @@ -58,14 +57,6 @@ const getSchemaFieldDefaultValue = (

const KNOWN_PROVIDER_DEFAULT_BASE_URLS: Partial<Record<string, Set<string>>> = {
openai: new Set(["https://api.openai.com", "https://api.openai.com/v1"]),
openhands: new Set([
OPENHANDS_LLM_PROXY_BASE_URL,
"https://llm-proxy.app.all-hands.dev/v1",
]),
litellm_proxy: new Set([
OPENHANDS_LLM_PROXY_BASE_URL,
"https://llm-proxy.app.all-hands.dev/v1",
]),
};

const normalizeBaseUrl = (baseUrl: string) => {
Expand Down Expand Up @@ -350,7 +341,6 @@ export function LlmSettingsScreen({
<>
<ModelSelector
currentModel={modelValue || undefined}
currentBaseUrl={baseUrlValue || undefined}
onChange={(provider, model) => {
const nextModel = buildModelId(provider, model);
if (nextModel) {
Expand Down
24 changes: 0 additions & 24 deletions src/utils/normalize-display-model.ts

This file was deleted.

22 changes: 0 additions & 22 deletions src/utils/openhands-llm.ts

This file was deleted.

Loading
Loading