mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: show 'Unset' for missing providers in AI models list (#27400)
## Summary
Frontend-only fixes for the `/ai/settings/models` page:
1. **Provider column displays "Unset"** with an info tooltip when a
model's provider has been deleted, instead of "N/A".
2. **Models without a usable provider display as "Disabled"** in the
list, regardless of the stored `enabled` flag. Covers both missing
(soft-deleted) and disabled providers.
3. **Save button re-enabled when only the provider changes** on the edit
page (previously the button stayed disabled because provider changes
lived outside the formik state).
## Scope
Frontend only. The DB constraint
`chat_model_configs_ai_provider_required_when_active` already prevents a
non-deleted model from having a NULL `ai_provider_id`; CODAGT-709
addresses the server-side cascade when a provider is deleted.
## Changes
- `ModelsPageView.tsx`: two `useMemo` maps (`hasProviderByModelId`,
`providerEnabledByModelId`) passed to `ModelRow`.
- `ModelRow.tsx`: `isEffectivelyEnabled = model.enabled && hasProvider
&& providerEnabled`. When `hasProvider` is false, renders "Unset" with a
standard `InfoIcon` tooltip.
- `ModelForm.tsx`: `canSubmit` OR's in `hasProviderChange` so the save
button enables when only the provider dropdown changes.
- `ModelRow.stories.tsx`: four stories covering baseline,
missing-provider (with tooltip assertion), disabled-provider, and
disabled-model paths.
- `ModelsPageView.stories.tsx`: `OrphanedModelShowsUnset` feeds an
orphaned model through the real derivation (map-miss + `?? false`),
matching the production shape produced by `deriveProviderStates`.
`DisabledProviderModelsStillListed` now asserts the "Disabled" badge.
- `ModelForm.stories.tsx`: `EditUpdateEnabledOnProviderChange` asserts
the save button is enabled when the selected provider differs from the
model's stored provider.
- `testFixtures.ts`: `mockOrphanedModel` fixture representing the
deleted-provider case.
Diff: 7 files, 240 insertions, 9 deletions.
> 🤖 This PR was updated with Coder Agents.
This commit is contained in:
@@ -154,6 +154,22 @@ Click the **star icon** next to a model in the models list to make it the
|
||||
default. The default model is pre-selected when developers start a new chat.
|
||||
Only one model can be the default at a time.
|
||||
|
||||
### Models with a missing or disabled provider
|
||||
|
||||
The Models list reflects whether each model can actually be used:
|
||||
|
||||
- When a model's connected provider has been deleted, the **Provider** column
|
||||
shows **Unset** with an info tooltip that reads "The provider connected to
|
||||
this model has been deleted."
|
||||
- When a model's provider is missing or disabled, the **Status** column
|
||||
shows **Disabled**, regardless of the model's own enabled setting. Such a
|
||||
model cannot serve chat requests.
|
||||
|
||||
To reconnect a model to a working provider, open the model from the list,
|
||||
pick a new provider from the **Provider** dropdown, and click **Save**. The
|
||||
Save button is enabled as soon as the selected provider differs from the
|
||||
model's current provider, even if no other field is edited.
|
||||
|
||||
## Model options
|
||||
|
||||
Every model has a set of general options and provider-specific options.
|
||||
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
mockClaude,
|
||||
mockDisabledModel,
|
||||
mockGPT5,
|
||||
mockOrphanedModel,
|
||||
mockProviderDisabledModel,
|
||||
} from "./testFixtures";
|
||||
|
||||
@@ -136,8 +137,36 @@ export const DisabledProviderModelsStillListed: Story = {
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await expect(canvas.getByText("GPT-4o Secondary")).toBeInTheDocument();
|
||||
await expect(canvas.getByText("OpenAI Secondary")).toBeInTheDocument();
|
||||
// useClickableTableRow renders each row with role="button", not "row",
|
||||
// so the row is queried by its clickable role.
|
||||
const row = canvas.getByRole("button", { name: /GPT-4o Secondary/i });
|
||||
await expect(within(row).getByText("OpenAI Secondary")).toBeInTheDocument();
|
||||
// A model under a disabled provider is not usable, so the status
|
||||
// column must show "Disabled" even though the stored enabled flag is
|
||||
// true. Scope to the target row so a fixture change cannot pass this
|
||||
// assertion via an unrelated "Disabled" cell.
|
||||
await expect(within(row).getByText("Disabled")).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
// An orphaned model is one whose ai_provider_id references a provider row
|
||||
// that has been deleted. In production `deriveProviderStates` drops such
|
||||
// models entirely, so the row reaches "Unset" via a map-miss and the
|
||||
// `?? false` fallback at ModelsPageView.tsx wiring. Reproduce that shape
|
||||
// here: the model appears in `models` but is not present in any
|
||||
// providerState.modelConfigs, so a `?? true` regression would flip this
|
||||
// story to "Enabled" and be caught.
|
||||
export const OrphanedModelShowsUnset: Story = {
|
||||
args: {
|
||||
models: [mockGPT5, mockOrphanedModel],
|
||||
providerStates: [MockOpenAIProviderState],
|
||||
providerTypeByID: new Map<string, string>([["prov-openai", "openai"]]),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const row = canvas.getByRole("button", { name: /Orphaned Model/i });
|
||||
await expect(within(row).getByText("Unset")).toBeInTheDocument();
|
||||
await expect(within(row).getByText("Disabled")).toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -135,6 +135,29 @@ const ModelsPageView: FC<ModelsPageViewProps> = ({
|
||||
return map;
|
||||
}, [providerStates]);
|
||||
|
||||
const hasProviderByModelId = useMemo(() => {
|
||||
const map = new Map<string, boolean>();
|
||||
for (const providerState of providerStates) {
|
||||
for (const providerModel of providerState.modelConfigs) {
|
||||
map.set(providerModel.id, Boolean(providerState.providerConfig));
|
||||
}
|
||||
}
|
||||
return map;
|
||||
}, [providerStates]);
|
||||
|
||||
const providerEnabledByModelId = useMemo(() => {
|
||||
const map = new Map<string, boolean>();
|
||||
for (const providerState of providerStates) {
|
||||
for (const providerModel of providerState.modelConfigs) {
|
||||
map.set(
|
||||
providerModel.id,
|
||||
providerState.providerConfig?.enabled === true,
|
||||
);
|
||||
}
|
||||
}
|
||||
return map;
|
||||
}, [providerStates]);
|
||||
|
||||
const filteredModels = useMemo(() => {
|
||||
const normalizedQuery = searchQuery.trim().toLowerCase();
|
||||
return models.filter((model) => {
|
||||
@@ -270,6 +293,10 @@ const ModelsPageView: FC<ModelsPageViewProps> = ({
|
||||
model={model}
|
||||
providerLabel={providerLabelByModelId.get(model.id) ?? ""}
|
||||
providerTypeByID={providerTypeByID}
|
||||
hasProvider={hasProviderByModelId.get(model.id) ?? false}
|
||||
providerEnabled={
|
||||
providerEnabledByModelId.get(model.id) ?? false
|
||||
}
|
||||
onClick={() => void navigate(`/ai/settings/models/${model.id}`)}
|
||||
/>
|
||||
))
|
||||
|
||||
@@ -280,6 +280,24 @@ export const EditUpdateDisabledUntilDirty: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
// Changing only the provider dropdown does not dirty the formik state
|
||||
// (providerKeyOverride lives outside form.values), so a naive form.dirty
|
||||
// gate leaves the save button disabled. `canSubmit` OR's in
|
||||
// `hasProviderChange` to fix this. Stripping that clause flips this story
|
||||
// red.
|
||||
export const EditUpdateEnabledOnProviderChange: Story = {
|
||||
args: {
|
||||
editingModel: mockGPT5,
|
||||
selectedProviderState: MockAnthropicProviderState,
|
||||
onDeleteModel: fn(async () => undefined),
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
const save = canvas.getByRole("button", { name: /^update model$/i });
|
||||
await expect(save).toBeEnabled();
|
||||
},
|
||||
};
|
||||
|
||||
export const ReasoningEffortInProviderConfiguration: Story = {
|
||||
args: {
|
||||
selectedProviderState: MockAnthropicProviderState,
|
||||
|
||||
@@ -227,13 +227,18 @@ export const ModelForm: FC<ModelFormProps> = ({
|
||||
const compressionThresholdValid =
|
||||
!form.values.compressionThreshold.trim() ||
|
||||
parseThresholdInteger(form.values.compressionThreshold) !== null;
|
||||
const hasProviderChange =
|
||||
isEditing &&
|
||||
!!editingModel &&
|
||||
!!selectedProviderState?.providerConfig &&
|
||||
selectedProviderState.providerConfig.id !== editingModel.ai_provider_id;
|
||||
const canSubmit =
|
||||
!isSaving &&
|
||||
!hasFieldErrors &&
|
||||
form.values.model.trim().length > 0 &&
|
||||
contextLimitValid &&
|
||||
compressionThresholdValid &&
|
||||
(!isEditing || form.dirty);
|
||||
(!isEditing || form.dirty || hasProviderChange);
|
||||
|
||||
const handleConfirmReplaceDefault = () => {
|
||||
replaceDefaultConfirmedRef.current = true;
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, userEvent, within } from "storybook/test";
|
||||
import { Table, TableBody } from "#/components/Table/Table";
|
||||
import { mockClaude, mockGPT5 } from "../testFixtures";
|
||||
import { ModelRow } from "./ModelRow";
|
||||
|
||||
const providerTypeByID = new Map<string, string>([
|
||||
["prov-openai", "openai"],
|
||||
["prov-anthropic", "anthropic"],
|
||||
]);
|
||||
|
||||
const meta: Meta<typeof ModelRow> = {
|
||||
title: "pages/AISettingsPage/ModelsPage/ModelRow",
|
||||
component: ModelRow,
|
||||
args: {
|
||||
model: mockGPT5,
|
||||
providerLabel: "OpenAI",
|
||||
providerTypeByID,
|
||||
hasProvider: true,
|
||||
providerEnabled: true,
|
||||
onClick: () => {},
|
||||
},
|
||||
render: (args) => (
|
||||
<Table>
|
||||
<TableBody>
|
||||
<ModelRow {...args} />
|
||||
</TableBody>
|
||||
</Table>
|
||||
),
|
||||
};
|
||||
|
||||
export default meta;
|
||||
type Story = StoryObj<typeof ModelRow>;
|
||||
|
||||
// Control case for the effective-status logic: when both `hasProvider` and
|
||||
// `providerEnabled` are true, the status badge must reflect the persisted
|
||||
// enabled flag as-is. Any regression that inverts this collapses every model
|
||||
// to "Disabled" in the list.
|
||||
export const WithProvider: Story = {
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await expect(canvas.getByText("OpenAI")).toBeInTheDocument();
|
||||
await expect(canvas.getByText("Enabled")).toBeInTheDocument();
|
||||
await expect(canvas.queryByText("Unset")).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
// When the provider is missing (soft-deleted or otherwise unavailable) the
|
||||
// Provider column shows "Unset" and the status collapses to "Disabled" even
|
||||
// though the persisted model.enabled flag is true. An info icon next to the
|
||||
// label reveals a tooltip explaining that the connected provider has been
|
||||
// deleted.
|
||||
export const WithoutProviderForcesDisabled: Story = {
|
||||
args: {
|
||||
model: { ...mockClaude, enabled: true },
|
||||
providerLabel: "",
|
||||
hasProvider: false,
|
||||
providerEnabled: false,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await expect(canvas.getByText("Unset")).toBeInTheDocument();
|
||||
await expect(canvas.getByText("Disabled")).toBeInTheDocument();
|
||||
await expect(canvas.queryByText("Enabled")).not.toBeInTheDocument();
|
||||
|
||||
const info = canvas.getByLabelText("Provider status");
|
||||
await userEvent.hover(info);
|
||||
const tooltip = await within(document.body).findByRole("tooltip");
|
||||
await expect(tooltip).toHaveTextContent(
|
||||
"The provider connected to this model has been deleted.",
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
// When the provider exists but is disabled, the label still renders (the
|
||||
// provider is set) but the status collapses to "Disabled" because the model
|
||||
// is not usable.
|
||||
export const DisabledProviderForcesDisabled: Story = {
|
||||
args: {
|
||||
model: { ...mockClaude, enabled: true, is_default: false },
|
||||
providerLabel: "Anthropic",
|
||||
hasProvider: true,
|
||||
providerEnabled: false,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await expect(canvas.getByText("Anthropic")).toBeInTheDocument();
|
||||
await expect(canvas.getByText("Disabled")).toBeInTheDocument();
|
||||
await expect(canvas.queryByText("Enabled")).not.toBeInTheDocument();
|
||||
await expect(canvas.queryByText("Unset")).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
// A disabled model with an enabled provider keeps its provider label but
|
||||
// stays "Disabled". This exercises the enabled=false path so the "Unset"
|
||||
// wording is only tied to the missing provider case.
|
||||
export const DisabledModelWithProvider: Story = {
|
||||
args: {
|
||||
model: { ...mockClaude, enabled: false, is_default: false },
|
||||
providerLabel: "Anthropic",
|
||||
hasProvider: true,
|
||||
providerEnabled: true,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
await expect(canvas.getByText("Anthropic")).toBeInTheDocument();
|
||||
await expect(canvas.getByText("Disabled")).toBeInTheDocument();
|
||||
await expect(canvas.queryByText("Unset")).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
@@ -1,9 +1,14 @@
|
||||
import { ChevronRightIcon } from "lucide-react";
|
||||
import { ChevronRightIcon, InfoIcon } from "lucide-react";
|
||||
import type { FC } from "react";
|
||||
import type { ChatModelConfig } from "#/api/typesGenerated";
|
||||
import { Avatar } from "#/components/Avatar/Avatar";
|
||||
import { Badge } from "#/components/Badge/Badge";
|
||||
import { TableCell, TableRow } from "#/components/Table/Table";
|
||||
import {
|
||||
Tooltip,
|
||||
TooltipContent,
|
||||
TooltipTrigger,
|
||||
} from "#/components/Tooltip/Tooltip";
|
||||
import { useClickableTableRow } from "#/hooks/useClickableTableRow";
|
||||
import { ProviderIcon } from "#/pages/AISettingsPage/ProvidersPage/components/ProviderIcon";
|
||||
|
||||
@@ -11,6 +16,8 @@ type ModelRowProps = {
|
||||
model: ChatModelConfig;
|
||||
providerLabel: string;
|
||||
providerTypeByID: ReadonlyMap<string, string>;
|
||||
hasProvider: boolean;
|
||||
providerEnabled: boolean;
|
||||
onClick: () => void;
|
||||
};
|
||||
|
||||
@@ -25,10 +32,15 @@ export const ModelRow: FC<ModelRowProps> = ({
|
||||
model,
|
||||
providerLabel,
|
||||
providerTypeByID,
|
||||
hasProvider,
|
||||
providerEnabled,
|
||||
onClick,
|
||||
}) => {
|
||||
const clickableProps = useClickableTableRow({ onClick });
|
||||
const displayName = model.display_name || model.model;
|
||||
// Models whose provider is missing or disabled cannot be used, so the
|
||||
// status column reflects that regardless of the persisted enabled flag.
|
||||
const isEffectivelyEnabled = model.enabled && hasProvider && providerEnabled;
|
||||
|
||||
return (
|
||||
<TableRow {...clickableProps}>
|
||||
@@ -58,12 +70,31 @@ export const ModelRow: FC<ModelRowProps> = ({
|
||||
</div>
|
||||
</TableCell>
|
||||
<TableCell className="min-w-0">
|
||||
<span
|
||||
className="block truncate text-sm font-medium leading-6 text-content-secondary"
|
||||
title={providerLabel}
|
||||
>
|
||||
{providerLabel || "N/A"}
|
||||
</span>
|
||||
{hasProvider ? (
|
||||
<span
|
||||
className="block truncate text-sm font-medium leading-6 text-content-secondary"
|
||||
title={providerLabel}
|
||||
>
|
||||
{providerLabel}
|
||||
</span>
|
||||
) : (
|
||||
<div className="flex items-center gap-1">
|
||||
<span className="truncate text-sm font-medium leading-6 text-content-secondary">
|
||||
Unset
|
||||
</span>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<InfoIcon
|
||||
aria-label="Provider status"
|
||||
className="size-3 text-content-secondary"
|
||||
/>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" className="max-w-[240px]">
|
||||
The provider connected to this model has been deleted.
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</div>
|
||||
)}
|
||||
</TableCell>
|
||||
<TableCell className="min-w-0">
|
||||
<span className="block truncate text-sm font-medium leading-6 text-content-secondary">
|
||||
@@ -72,7 +103,7 @@ export const ModelRow: FC<ModelRowProps> = ({
|
||||
</TableCell>
|
||||
<TableCell>
|
||||
<Badge variant="default">
|
||||
{model.enabled ? "Enabled" : "Disabled"}
|
||||
{isEffectivelyEnabled ? "Enabled" : "Disabled"}
|
||||
</Badge>
|
||||
</TableCell>
|
||||
<TableCell className="w-10 text-center">
|
||||
|
||||
@@ -144,3 +144,17 @@ export const MockCopilotProviderState: ProviderState = {
|
||||
},
|
||||
modelConfigs: [],
|
||||
};
|
||||
|
||||
// A model whose provider row has been deleted. In production such models
|
||||
// still appear in the top-level model list, but `deriveProviderStates`
|
||||
// drops them from every providerState.modelConfigs. Stories should feed
|
||||
// this fixture through `models` alone; do not add it to a provider state.
|
||||
export const mockOrphanedModel: ChatModelConfig = {
|
||||
...mockGPT5,
|
||||
id: "model-orphaned",
|
||||
ai_provider_id: "prov-orphaned",
|
||||
model: "gpt-4o-orphaned",
|
||||
display_name: "Orphaned Model",
|
||||
is_default: false,
|
||||
enabled: true,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user