fix(providers): apply the review of PR #988

Five reviewers (Fable 5.1, Opus 5.5, Sonnet 5.5, Codex GPT-6 Astra, Codex
GPT-6.1 Sol) found no behavior or security regression; this commit applies
what they asked for.

Settings dialog: the automatic model list fetch only fills the list, which
waits behind 'Browse models'; it no longer opens the picker (the modal picker
took the focus from whatever the user was typing) and shows no error of its
own. One list request per provider at a time, so a click during the automatic
fetch sends nothing twice; the baseline that detects freshly entered
credentials is taken on every visit of a provider page; SGLang's prefilled
local example URL triggers no fetch. Entries saved with Azure's old
placeholder URL still count as having no URL. The base URL field of Azure and
of the custom preset is labelled required (new dictionary key baseUrlRequired).

Smaller code: the PROVIDER_INFO alias is gone (three files and the tests
import PROVIDERS), as are three logo values no code read, two unused type
re-exports, three unused exports and SINGLE_SYSTEM_PROVIDERS (the chat route
reads the record). Renamed for clarity: REQUIRED_KEY_VARS, requiredKeyEnv,
configuredUrl, LocalizedText. Seven stale comments rewritten; the custom
preset's note shortened.

Tests: the matrix gains six override shapes (a Vertex key alone, AWS keys with
a session token, headers with a URL, a trusted URL with a key, a key with a
server model's URL variable, '/' alone), 5843 rows; the new rows were
regenerated on the pre-refactor code and match it. The matrix and the server
credential tests clear every variable they read; the admin variable table
covers every provider; a test that derived its expectation from the function
under test is gone; the singleSystem, listNeedsNoKey, sdkKnowsEndpoint and
auto-detection sets are spelled out.

Docs: the READMEs say 25 providers and name the OpenAI-compatible preset; the
AI_PROVIDER lists are complete; the Azure example URL has its /openai suffix;
'Adding a Provider' names the tests to update.
This commit is contained in:
dayuan.jiang
2026-10-12 21:17:51 +09:00
parent 65625ba212
commit 75ffc32a49
29 changed files with 1880 additions and 161 deletions
+44 -15
View File
@@ -55,7 +55,7 @@ import { getApiEndpoint } from "@/lib/base-path"
import { formatMessage } from "@/lib/i18n/utils"
import type { ListedModel } from "@/lib/provider-models"
import { hasCredentials } from "@/lib/provider-setup"
import { PROVIDERS } from "@/lib/providers"
import { PROVIDERS, publicDefaultUrl } from "@/lib/providers"
import { STORAGE_KEYS } from "@/lib/storage"
import type {
ModelConfig,
@@ -306,13 +306,30 @@ export function ModelConfigDialog({
model_not_found: t.modelNotFound,
})
const handleFetchModels = async () => {
// Requests in flight, by provider id: a second request for the same
// provider (the button clicked while the automatic fetch runs, or the
// reverse) is not sent
const inFlight = useRef(new Set<string>())
// The credentials each provider's automatic fetch ran with, or was
// overtaken by a click with: one automatic fetch per set of credentials
const autoFetched = useRef<Record<string, string>>({})
/**
* Fetch the shown provider's model list. A click opens the picker with
* the result and shows what went wrong; the automatic fetch after
* credentials were entered (auto) only fills the list, so it never
* takes the focus or flashes an error while the user is still typing.
*/
const handleFetchModels = async (auto = false) => {
if (!selectedProvider) return
const providerId = selectedProvider.id
if (inFlight.current.has(providerId)) return
const askedWith = credentialsOf(providerId)
autoFetched.current[providerId] = askedWith
const hints = errorHints(selectedProvider)
inFlight.current.add(providerId)
setFetchingFor((current) => [...current, providerId])
setFetchModelsError("")
if (!auto) setFetchModelsError("")
try {
const response = await fetch(
getApiEndpoint("/api/provider-models"),
@@ -339,11 +356,11 @@ export function ModelConfigDialog({
...current,
[providerId]: { askedWith, models: data.models },
}))
if (!stillShown) return
if (!stillShown || auto) return
if (data.models.length > 0) setModelPickerOpen(true)
else setFetchModelsError(t.noModelsReturned)
} else {
if (!stillShown) return
if (!stillShown || auto) return
setFetchModelsError(
[hints[data.code], data.error].filter(Boolean).join(" ") ||
formatMessage(t.requestFailed, {
@@ -353,12 +370,14 @@ export function ModelConfigDialog({
}
} catch {
if (
!auto &&
selectedProviderIdRef.current === providerId &&
credentialsOf(providerId) === askedWith
) {
setFetchModelsError(dict.errors.networkError)
}
} finally {
inFlight.current.delete(providerId)
setFetchingFor((current) =>
current.filter((id) => id !== providerId),
)
@@ -406,23 +425,33 @@ export function ModelConfigDialog({
.slice(0, 4)
// A provider that can list its models gets its list fetched by itself,
// once, for credentials entered on its page while it has no model yet:
// the user picks from the list instead of a hand-written one. Credentials
// a provider already had when its page opened fetch nothing (the button
// does), nor does a provider with models.
// once, for credentials entered on its page while it has no model yet,
// so "Browse models" offers the provider's own list instead of a
// hand-written one. Credentials a provider already had when its page
// opened fetch nothing (the button does), nor does a provider with
// models, nor a local example URL still in the field (SGLang's).
const shownWith = useRef<{ id: string; credentials: string } | null>(null)
const autoFetched = useRef<Record<string, string>>({})
useEffect(() => {
if (!open || !selectedProvider) return
if (!open || !selectedProvider) {
// The baseline belongs to one visit of one provider's page
shownWith.current = null
return
}
const id = selectedProvider.id
const now = credentialsOf(id)
if (shownWith.current?.id !== id) {
shownWith.current = { id, credentials: now }
}
const def = PROVIDERS[selectedProvider.provider]
const localExample =
!!selectedProvider.baseUrl &&
selectedProvider.baseUrl === def.defaultBaseUrl &&
!publicDefaultUrl(selectedProvider.provider)
if (
!PROVIDERS[selectedProvider.provider].modelList ||
!def.modelList ||
selectedProvider.models.length > 0 ||
!hasCredentials(selectedProvider) ||
localExample ||
now === shownWith.current.credentials ||
autoFetched.current[id] === now
) {
@@ -430,8 +459,8 @@ export function ModelConfigDialog({
}
// Once the typing or pasting pauses
const timer = setTimeout(() => {
autoFetched.current[id] = now
void handleFetchModels()
if (autoFetched.current[id] === now) return
void handleFetchModels(true)
}, 600)
return () => clearTimeout(timer)
}, [open, selectedProvider])
@@ -1064,7 +1093,7 @@ export function ModelConfigDialog({
def.modelList && (
<button
type="button"
onClick={handleFetchModels}
onClick={() => handleFetchModels()}
disabled={fetchingFor.includes(
provider.id,
)}
+7 -3
View File
@@ -92,9 +92,13 @@ export function ProviderCredentialsFields({
const dict = useDictionary()
const locale = useLocale()
const info = PROVIDERS[provider]
const baseUrlLabel = formatMessage(dict.modelConfig.baseUrlWithExample, {
example: info.defaultBaseUrl || "https://api.example.com/v1",
})
// A key-only provider without a default URL (Azure, custom) needs one
const baseUrlLabel =
info.credential === "apiKey" && !info.defaultBaseUrl
? dict.modelConfig.baseUrlRequired
: formatMessage(dict.modelConfig.baseUrlWithExample, {
example: info.defaultBaseUrl || "https://api.example.com/v1",
})
const requestUrl = baseUrl ? chatRequestUrl(provider, baseUrl) : null
// Open when it holds something the user set, or for a provider whose
// base URL usually needs changing or comes with a hint
+8 -9
View File
@@ -16,11 +16,11 @@ import { ScrollArea } from "@/components/ui/scroll-area"
import { useDictionary } from "@/hooks/use-dictionary"
import { formatMessage } from "@/lib/i18n/utils"
import { POPULAR_PROVIDERS, providerStatus } from "@/lib/provider-setup"
import {
type FlattenedModel,
PROVIDER_INFO,
type ProviderConfig,
type ProviderName,
import { PROVIDERS } from "@/lib/providers"
import type {
FlattenedModel,
ProviderConfig,
ProviderName,
} from "@/lib/types/model-config"
import { cn } from "@/lib/utils"
@@ -169,7 +169,7 @@ export function ProvidersList({
/>
</span>
<span className="truncate">
{PROVIDER_INFO[provider].label}
{PROVIDERS[provider].label}
</span>
</button>
))}
@@ -180,8 +180,7 @@ export function ProvidersList({
className="flex items-center gap-1 text-[13px] font-medium hover:underline"
>
{formatMessage(t.viewAllProviders, {
count: Object.keys(PROVIDER_INFO)
.length,
count: Object.keys(PROVIDERS).length,
})}
<ChevronRight className="size-3.5" />
</button>
@@ -225,7 +224,7 @@ export function ProvidersList({
<span className="flex items-center gap-2 text-[13px] font-medium">
<span className="truncate">
{p.name ||
PROVIDER_INFO[
PROVIDERS[
p.provider
].label}
</span>