fix: address review feedback on model config workflow

- Send explicit {} for empty extra_body/custom_headers fields so the
  backend clears stored values instead of preserving them
- Merge backend provider_options with frontend PROVIDERS registry so
  the provider picker reflects backend-supported providers and policy
  fields (create_allowed, default_auth_method, auth_method_locked)
- Render provider combobox popover inside the sheet scroll container
  to fix wheel events scrolling the sheet instead of the provider list
This commit is contained in:
SiYue-ZO 2026-05-08 14:14:58 +08:00
parent bcf05e5522
commit c837955d11
6 changed files with 130 additions and 13 deletions

View file

@ -6,7 +6,12 @@ import {
import { useCallback, useEffect, useRef, useState } from "react"
import { useTranslation } from "react-i18next"
import { addModel, getCatalogs, setDefaultModel } from "@/api/models"
import {
type ModelProviderOption,
addModel,
getCatalogs,
setDefaultModel,
} from "@/api/models"
import { ConfigChangeNotice } from "@/components/config-change-notice"
import { maskedSecretPlaceholder } from "@/components/secret-placeholder"
import {
@ -111,6 +116,7 @@ interface AddModelSheetProps {
onClose: () => void
onSaved: () => void
existingModelNames: string[]
providerOptions?: ModelProviderOption[]
}
export function AddModelSheet({
@ -118,6 +124,7 @@ export function AddModelSheet({
onClose,
onSaved,
existingModelNames,
providerOptions,
}: AddModelSheetProps) {
const { t } = useTranslation()
const [form, setForm] = useState<AddForm>(EMPTY_ADD_FORM)
@ -134,6 +141,7 @@ export function AddModelSheet({
const [fetchedModels, setFetchedModels] = useState<string[]>([])
const [catalogModels, setCatalogModels] = useState<string[]>([])
const debounceRef = useRef<ReturnType<typeof setTimeout>>(undefined)
const scrollContainerRef = useRef<HTMLDivElement>(null)
const apiKeyPlaceholder = maskedSecretPlaceholder(
form.apiKey,
t("models.field.apiKeyPlaceholder"),
@ -283,6 +291,8 @@ export function AddModelSheet({
try {
if (form.extraBody.trim()) {
extraBody = JSON.parse(form.extraBody.trim())
} else {
extraBody = {}
}
} catch {
setServerError(
@ -293,6 +303,8 @@ export function AddModelSheet({
try {
if (form.customHeaders.trim()) {
customHeaders = JSON.parse(form.customHeaders.trim())
} else {
customHeaders = {}
}
} catch {
setServerError(
@ -362,7 +374,7 @@ export function AddModelSheet({
</SheetDescription>
</SheetHeader>
<div className="min-h-0 flex-1 overflow-y-auto">
<div className="min-h-0 flex-1 overflow-y-auto" ref={scrollContainerRef}>
<div className="space-y-5 px-6 py-5">
<Field
label={t("models.add.modelName")}
@ -389,6 +401,9 @@ export function AddModelSheet({
value={form.provider}
onChange={handleProviderChange}
placeholder={t("models.field.providerPlaceholder")}
backendOptions={providerOptions}
filterCreateAllowed
containerRef={scrollContainerRef}
/>
</Field>

View file

@ -8,6 +8,7 @@ import { useTranslation } from "react-i18next"
import {
type ModelInfo,
type ModelProviderOption,
getCatalogs,
setDefaultModel,
updateModel,
@ -65,6 +66,7 @@ interface EditModelSheetProps {
open: boolean
onClose: () => void
onSaved: () => void
providerOptions?: ModelProviderOption[]
}
function normalizeApiBase(value: string): string {
@ -126,6 +128,7 @@ export function EditModelSheet({
open,
onClose,
onSaved,
providerOptions,
}: EditModelSheetProps) {
const { t } = useTranslation()
const [form, setForm] = useState<EditForm>({
@ -155,6 +158,7 @@ export function EditModelSheet({
const [fetchedModels, setFetchedModels] = useState<string[]>([])
const [catalogModels, setCatalogModels] = useState<string[]>([])
const debounceRef = useRef<ReturnType<typeof setTimeout>>(undefined)
const scrollContainerRef = useRef<HTMLDivElement>(null)
const initialForm = model ? buildInitialEditForm(model) : null
const isDirty =
model != null &&
@ -256,6 +260,8 @@ export function EditModelSheet({
try {
if (form.extraBody.trim()) {
extraBody = JSON.parse(form.extraBody.trim())
} else {
extraBody = {}
}
} catch {
setError(
@ -266,6 +272,8 @@ export function EditModelSheet({
try {
if (form.customHeaders.trim()) {
customHeaders = JSON.parse(form.customHeaders.trim())
} else {
customHeaders = {}
}
} catch {
setError(
@ -343,7 +351,7 @@ export function EditModelSheet({
</SheetDescription>
</SheetHeader>
<div className="min-h-0 flex-1 overflow-y-auto">
<div className="min-h-0 flex-1 overflow-y-auto" ref={scrollContainerRef}>
<div className="space-y-5 px-6 py-5">
<Field
label={t("models.field.provider")}
@ -353,6 +361,8 @@ export function EditModelSheet({
value={form.provider}
onChange={handleProviderChange}
placeholder={t("models.field.providerPlaceholder")}
backendOptions={providerOptions}
containerRef={scrollContainerRef}
/>
</Field>

View file

@ -8,7 +8,12 @@ import { useCallback, useEffect, useState } from "react"
import { useTranslation } from "react-i18next"
import { toast } from "sonner"
import { type ModelInfo, getModels, setDefaultModel } from "@/api/models"
import {
type ModelInfo,
type ModelProviderOption,
getModels,
setDefaultModel,
} from "@/api/models"
import { PageHeader } from "@/components/page-header"
import { Button } from "@/components/ui/button"
import { showSaveSuccessOrRestartToast } from "@/lib/restart-required"
@ -33,6 +38,9 @@ interface ProviderGroup {
export function ModelsPage() {
const { t } = useTranslation()
const [models, setModels] = useState<ModelInfo[]>([])
const [providerOptions, setProviderOptions] = useState<
ModelProviderOption[]
>([])
const [loading, setLoading] = useState(true)
const [fetchError, setFetchError] = useState("")
@ -55,6 +63,7 @@ export function ModelsPage() {
return a.model_name.localeCompare(b.model_name)
})
setModels(sorted)
setProviderOptions(data.provider_options || [])
setFetchError("")
} catch (e) {
setFetchError(e instanceof Error ? e.message : t("models.loadError"))
@ -200,6 +209,7 @@ export function ModelsPage() {
open={editingModel !== null}
onClose={() => setEditingModel(null)}
onSaved={fetchModels}
providerOptions={providerOptions}
/>
<AddModelSheet
@ -207,6 +217,7 @@ export function ModelsPage() {
onClose={() => setAddOpen(false)}
onSaved={fetchModels}
existingModelNames={models.map((model) => model.model_name)}
providerOptions={providerOptions}
/>
<DeleteModelDialog

View file

@ -20,27 +20,52 @@ import {
import { cn } from "@/lib/utils"
import { ProviderIcon } from "./provider-icon"
import { KNOWN_PROVIDER_KEYS, PROVIDERS } from "./provider-registry"
import {
type MergedProvider,
PROVIDERS,
mergeWithBackendOptions,
} from "./provider-registry"
import type { ModelProviderOption } from "@/api/models"
interface ProviderComboboxProps {
value: string
onChange: (value: string) => void
placeholder?: string
backendOptions?: ModelProviderOption[]
/** When true, only show providers with create_allowed from the backend. */
filterCreateAllowed?: boolean
/** Container element for the popover portal. Use to avoid scroll conflicts inside dialogs/sheets. */
containerRef?: React.RefObject<HTMLElement | null>
}
export function ProviderCombobox({
value,
onChange,
placeholder,
backendOptions,
filterCreateAllowed,
containerRef,
}: ProviderComboboxProps) {
const { t } = useTranslation()
const [open, setOpen] = useState(false)
const [customMode, setCustomMode] = useState(false)
const [customValue, setCustomValue] = useState("")
const sorted = [...PROVIDERS].sort((a, b) => b.priority - a.priority)
const selected = sorted.find((p) => p.key === value)
const isCustom = value && !KNOWN_PROVIDER_KEYS.has(value)
const allProviders: MergedProvider[] = backendOptions
? mergeWithBackendOptions(backendOptions)
: [...PROVIDERS]
.sort((a, b) => b.priority - a.priority)
.map((p) => ({
...p,
createAllowed: true,
defaultModelAllowed: false,
}))
const visible = filterCreateAllowed
? allProviders.filter((p) => p.createAllowed)
: allProviders
const allKeys = new Set(allProviders.map((p) => p.key))
const selected = allProviders.find((p) => p.key === value)
const isCustom = value && !allKeys.has(value)
const handleSelect = (currentValue: string) => {
if (currentValue === "__custom__") {
@ -97,7 +122,7 @@ export function ProviderCombobox({
<IconChevronDown className="ml-2 size-4 shrink-0 opacity-50" />
</Button>
</PopoverTrigger>
<PopoverContent className="w-[--radix-popover-trigger-width] p-0">
<PopoverContent className="w-[--radix-popover-trigger-width] p-0" container={containerRef?.current}>
{customMode ? (
<div className="flex flex-col gap-2 p-2">
<Input
@ -142,7 +167,7 @@ export function ProviderCombobox({
<CommandList>
<CommandEmpty>{t("models.combobox.noProvider")}</CommandEmpty>
<CommandGroup>
{sorted.map((provider) => (
{visible.map((provider) => (
<CommandItem
key={provider.key}
value={provider.key}

View file

@ -4,6 +4,8 @@
* should derive their data from this registry.
*/
import type { ModelProviderOption } from "@/api/models"
export interface ProviderDefinition {
key: string
label: string
@ -464,3 +466,55 @@ function editDistance(a: string, b: string): number {
}
return dp[m][n]
}
// ── Backend options merge ────────────────────────────────────────────────────
export interface MergedProvider extends ProviderDefinition {
createAllowed: boolean
defaultModelAllowed: boolean
defaultAuthMethod?: string
authMethodLocked?: boolean
}
/**
* Merge the frontend PROVIDERS registry with backend provider_options.
* Frontend provides presentation data (labels, icons, priority, etc.).
* Backend provides authoritative availability and policy fields.
*/
export function mergeWithBackendOptions(
backendOptions: ModelProviderOption[],
): MergedProvider[] {
const backendMap = new Map(backendOptions.map((o) => [o.id, o]))
const merged: MergedProvider[] = []
// Start with frontend providers, enriched with backend policy
for (const p of PROVIDERS) {
const backend = backendMap.get(p.key)
merged.push({
...p,
createAllowed: backend?.create_allowed ?? false,
defaultModelAllowed: backend?.default_model_allowed ?? false,
defaultAuthMethod: backend?.default_auth_method,
authMethodLocked: backend?.auth_method_locked,
})
if (backend) backendMap.delete(p.key)
}
// Add providers only known to the backend
for (const [key, backend] of backendMap) {
merged.push({
key,
label: key,
requiresApiKey: !backend.empty_api_key_allowed,
isLocal: backend.empty_api_key_allowed,
priority: 0,
createAllowed: backend.create_allowed,
defaultModelAllowed: backend.default_model_allowed,
defaultAuthMethod: backend.default_auth_method,
authMethodLocked: backend.auth_method_locked,
defaultApiBase: backend.default_api_base || undefined,
})
}
return merged.sort((a, b) => b.priority - a.priority)
}

View file

@ -9,9 +9,11 @@ const PopoverTrigger = PopoverPrimitive.Trigger
const PopoverContent = React.forwardRef<
React.ComponentRef<typeof PopoverPrimitive.Content>,
React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content>
>(({ className, align = "center", sideOffset = 4, ...props }, ref) => (
<PopoverPrimitive.Portal>
React.ComponentPropsWithoutRef<typeof PopoverPrimitive.Content> & {
container?: HTMLElement | null
}
>(({ className, align = "center", sideOffset = 4, container, ...props }, ref) => (
<PopoverPrimitive.Portal container={container}>
<PopoverPrimitive.Content
ref={ref}
align={align}