feat(model-check): unify provider model checks

[AtomsH4]

Branch strategy

• Active development targets main.

What this PR does

Before this PR:

Provider settings exposed separate connection-check and health-check entry points. Single-model checks started beside the API key field, while all-model configuration and results lived in a separate drawer, making the full workflow harder to discover and act on.

After this PR:

Provider settings expose one Model Check action in the model-list toolbar. The dialog preserves the familiar single-model connection-check content and styling from main. Selecting Check all models replaces the content of the same dialog with the all-model form while retaining its existing submission and progress-reporting behavior.

The single-model and all-model API key selectors now share the same masked-key-only options, search behavior, and styling. The “Select the API key to use” label no longer has a trailing colon.

All-model progress appears beside each affected model. Result details include per-key outcomes and enablement controls, while costly generation and speech models are skipped with an explicit reason. Retained successful key results show latency rounded to two decimal places, and disabling a key keeps its result card and switch without a redundant disabled-status row.

Fixes #17935, fixes #18434

Why we need it and why it was done in this way

The following tradeoffs were made:

The single-model and all-model runners remain separate because their observable lifecycles differ: single-model failures stay in the dialog, while all-model checks close the dialog and stream results into model rows. The two workflows share one dialog shell, with Check all models replacing its content instead of introducing another modal or restoring the former drawer.

Reports remain renderer-local and are not persisted across provider changes or application restarts. Generation and speech models are skipped instead of extending the probe infrastructure and potentially issuing expensive or unreliable requests.

The following alternatives were considered:

Keeping the existing entry points and results drawer was rejected because it preserves the discoverability and actionability problems reported in the linked issues. Collapsing both workflows into one runner was rejected because it would couple different completion and cancellation contracts. Replacing the familiar single-model connection-check layout was rejected because it introduced unnecessary UI differences. Extending the main-process probe API to generation and speech models was rejected as a separate, higher-risk feature outside this renderer-only change.

Links to places where the discussion took place: #17935, #18434

Breaking changes

The former API-key-row connection-check action and all-model health-check drawer are removed. Users now start both workflows through Model Check in the provider model-list toolbar, and all-model results appear inline beside each model. No schema, migration, IPC, or persisted-data changes are required.

Special notes for your reviewer

API key enablement changes preserve the current model-check report, and the retained result reflects the key's latest enabled state.

Model-check status labels and startup-failure feedback use i18n. Provider-supplied diagnostic error text remains verbatim so the response body remains useful for troubleshooting.

The single-model dialog matches the existing connection-check presentation. Its Check all models action replaces the current dialog content with the retained all-model form. Both API key selectors use the same component and expose only masked key values rather than key labels.

Verification for the latest dialog follow-up:

• pnpm vitest run --project renderer src/renderer/pages/settings/ProviderSettings/ModelList/__tests__/ModelCheckDialog.test.tsx (10 tests passed)
• pnpm typecheck:web
• pnpm i18n:check
• Changed-file ESLint and Biome checks
• Interactive Electron verification of both selectors, dialog replacement, option content, styling, and label punctuation

The full test suite was not rerun for this localized follow-up.

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

• Branch: This PR targets main
• PR: The PR description is expressive enough and will help future contributors
• Code: Write code that humans can understand and Keep it simple
• Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
• Upgrade: Impact of this change on upgrade flows was considered and addressed if required
• Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior.
• Self-review: I have reviewed my own code (e.g., via /gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others

Release note

Provider settings now provide one Model Check action for checking a single model or all models. The familiar single-model connection-check dialog is preserved, while Check all models switches the same dialog to the bulk-check form. Both workflows use consistent masked API key selectors. All-model progress and per-API-key results appear beside each model, while costly generation and speech models are skipped with an explanation.

[claude]

Read-only review complete. Inspected the unified Model Check orchestration hooks (useHealthCheck, useProviderConnectionCheck, modelListHealthContext), the new dialog/result UI (ModelCheckDialog, ApiKeyCheckResults, ModelCheckStatus, ProviderModelCheck), the combobox accessibility change, and the deletion boundaries.

No high-confidence issues found. The run/cancel/runId machinery is consistent across both hooks (no stuck isChecking paths), the credential-fingerprint effect correctly spares an in-flight run it just committed, toggling a key's enabled state deliberately excludes isEnabled from the fingerprint so displayed results are preserved, and every abortInFlightCheck call site performs the matching state reset. The two useModels({ providerId }) instances share the SWR cache key, so dialog models and row statuses use identical ids. Deleted modules leave no dangling imports, and all referenced i18n keys exist.

[DeJeune]

Solid unification — the ModelCheckCredential abstraction is a real improvement over the old string[] key plumbing, the run-id + AbortController staleness guards are handled carefully, deliberately excluding isEnabled from the credential fingerprint so a toggled key keeps its result card is a nice touch, i18n is complete in both locales (including the _one/_other plurals), and no dangling references to the deleted drawers remain. Tests are contract-shaped rather than behaviour-pinned.

Two things I would like addressed before merge:

• Orphaned surface (B3). The removal left useHealthCheck's apiKeyEntries / requiresApiKey returns, resetHealthCheckRun, the useModelListHealth barrel export, and several now-unconstructible types/healthCheck.ts variants behind with zero consumers.
• Duplicated credential preparation (B2). getRefetchedApiKeyEntries, createCredentialFingerprint, and the whole preparing/accepted-fingerprint abort dance are byte-identical across useHealthCheck.ts and useProviderConnectionCheck.ts. It is the subtlest logic in the feature and it now lives in two places.

The remaining four comments (stale dialog state on reopen, mode reset on model-list change, segmented-control accessible name, stale guard message) are smaller and fine to fold in or push back on.

Checked and cleared, for the record: apiKeyOverride: '' on the provider-auth credential is safe — resolveApiKey only reaches it for withProviderAuth providers (copilot / codex / grok-cli / Vertex / Bedrock, where the override is ignored) and for auth-optional providers with zero enabled keys, where undefined resolves identically. Deleting components/__tests__/AuthenticationSection.test.tsx is correct; it was mock-prop-spy assertions against the removed drawer.

[DeJeune]

Follow-up pass on 7140c564, this time specifically through the SWR / React-performance lens (vercel-react-best-practices). Both findings are non-blocking; my earlier REQUEST_CHANGES items are all resolved.

Two small ones, inline:

• useProvider → useProviderById in useModelCheckCredentials and useHealthCheck (12 idle useSWRMutation instances, 6 of them new in the refactor).
• Key-selection sanitization in ModelCheckDialog is an Effect where a render-time derivation would do.

What I checked and found correct, since these are the parts of this design most likely to go wrong:

• SWR keys are stable. useModels rebuilds its omitBy(query, isUndefined) object every render, but buildSWRKey produces an array key [path, query] that SWR serializes with stableHash — no duplicate fetches from the two runner hooks.
• refetchApiKeys() in prepareCredentials is not a redundant round trip. The async callback needs the post-write value, and the closed-over apiKeyEntries is a stale render snapshot; using the bound mutate() return value is the right SWR pattern. Worth keeping as is.
• The run/results context split does its job. ModelCheckDialog stays mounted while closed but only subscribes to useModelListHealthRun(), so streamed per-model results do not re-render it or the toolbar.
• No O(n²) render cost during an all-model check. ModelListSections renders through DynamicVirtualList with overscan: 10, so each status update reconciles ~25 rows, and memo(ModelListItem) bails out for every row whose modelStatus reference is unchanged. The per-update new Map(...) rebuild is milliseconds even at 500 models.
• toggleApiKey does not defeat the runValue memo. useMutation reads its options through a ref, so the freshly built refresh array does not churn trigger identity, and updateApiKey stays stable.
• refresh scope is right. providerRefreshPaths is ["/providers", "/providers/{id}", "/providers/{id}/*"] — it does not touch /models, so enabling/disabling a key from a result card cannot churn the model list or perturb an in-flight check.
• No focus-revalidation churn. DEFAULT_SWR_OPTIONS disables revalidateOnFocus and revalidateOnReconnect.

[DeJeune]

Verification notes for 7140c564 — the record behind "all six resolved", posted so you can see what was actually re-derived rather than taken on faith. (The SWR/React follow-up I left separately is additive and non-blocking.)

On the two blockers

Orphaned surface + duplication. The extraction is the right shape. useModelCheckCredentials.ts now solely owns the provider / API-key / auth / meta subscriptions, the commitInputApiKeyNow → refetchApiKeys → resolveModelCheckCredentials sequence, and the preparingCredentialsRef / acceptedCredentialFingerprintRef invalidation dance — surfaced to both runners as a credentialChangeVersion counter plus prepareCredentials(selection, signal). Each runner keeps only its own abort + state reset, which is genuinely per-runner rather than shared logic left behind. useHealthCheck dropped ~100 lines, useProviderConnectionCheck ~97. Dead surface confirmed gone by repo-wide grep: resetHealthCheckRun, useModelListHealth (function and barrel export), and the idle / checking variants plus the model?: Model field in types/healthCheck.ts.

The other four. Mode reset split into two effects, with a regression test that holds All models and a timeout of 27 across a model-list change; openModelCheck clears the prior result, with a test asserting a stale failed becomes none on reopen; the mode control uses the new settings.models.check.model_scope label (present in both locales) and DialogHeader lost the reused id; the guard message now names ApiKeyProvider.

What I re-derived in the refactor

• Error classification is behaviour-preserving. ModelCheckCredentialsSaveError replaces the didCommitApiKey boolean, and the runners deliberately log without toasting. I first read that as a silent-failure regression — it isn't: commitInputApiKeyNow (useProviderApiKey.ts:222) already toasts api_key.save_failed and rethrows, so a second toast would be the duplicate you removed earlier in this branch. A refetchApiKeys failure still falls through to check.failed_to_start, unchanged.
• Abort semantics survive the move. prepareCredentials now throws via signal.throwIfAborted() where the old code returned early on runId !== runIdRef.current || signal.aborted. Equivalent, because abortInFlightCheck always aborts the controller before bumping runId — so "superseded" implies "aborted", and both catch blocks return silently on signal.aborted.
• The now-shared preparingCredentialsRef is safe. One ref serving two runners is only sound because the context guards on isHealthChecking || isSingleModelChecking; with that guard the two prepareCredentials calls can never overlap. Worth keeping in mind if a future change ever allows a single-model check to start during an all-model run.
• The deliberate retention behaviour still holds. createCredentialFingerprint still excludes isEnabled, so toggling a key from a result card does not bump credentialChangeVersion and does not discard the report.
• Effect ordering across the three hooks. All three live in ModelListHealthProvider, so a version bump from the credentials hook lands one render before the runners' abort effects observe it. A tick's delay, no correctness impact.

CI

Fully green now, where two renderer shards were still pending at my first pass: basic-checks, all five renderer-test shards, package-test, general-test, render-test, translate.

One residual, deliberately not filed

Both runners still independently call useModels / useProvider / useProviderEndpoints and keep their own endpoint-change abort effect. That is per-runner reset state, so leaving it is defensible — hoisting it would be churn for no behavioural gain. (The useProvider half of that is the one thing I did file, in the follow-up review, and only because the mutation bundle is unused.)

[zhangjiadi225]

LGTM. The follow-up review feedback has been addressed and the latest checks are passing.

[claude]

Reviewed this PR for correctness, async handling, error propagation, and state consistency. The implementation correctly handles abort signals through the async chain, properly coordinates single-model and all-model check states, and has appropriate error handling for credential preparation and check failures. No blocking issues found.

[claude]

Warning: Misleading error message for disabled API key

When resolveModelCheckCredentials throws 'api_key_unavailable' for a single-selected key not found in enabledEntries, the user sees a generic "enter API key" message (message.error.enter.api.label). This is misleading because:

1. The key may exist but be disabled (filtered out by isEnabled)
2. The key may have been deleted after the dialog opened

Failure scenario: User disables an API key, then immediately tries to check a model with that key selected in single-mode. The error says "enter API key" but the key already exists and is disabled.

Suggested fix: Add a new error code (e.g., 'api_key_disabled') when the key exists but is disabled, and show a more specific message like "Selected API key is disabled. Enable it or select a different key."

See also: useProviderConnectionCheck.ts:121-122 where this error is caught and the generic message is shown.

[claude]

Notice: Behavior-pinning test

This test asserts the exact English output string after i18n.changeLanguage('en-US'). This is a behavior-pinning test that:

1. Re-derives what the implementation produces rather than asserting the contract
2. Will fail spuriously if translation strings change or are refined

Suggested fix: The test should verify the i18n contract by mocking i18n.t() and asserting:

1. Correct keys are called ('settings.models.check.model_status_passed', etc.)
2. Correct count parameters are passed
3. Keys are combined correctly

This makes the test more resilient to translation changes while still catching real bugs in the summarization logic.