Skip to content

feat(model manager): external providers UI/UX tweaks#9237

Open
joshistoast wants to merge 4 commits into
invoke-ai:mainfrom
joshistoast:feat/api-keys-ui-redux
Open

feat(model manager): external providers UI/UX tweaks#9237
joshistoast wants to merge 4 commits into
invoke-ai:mainfrom
joshistoast:feat/api-keys-ui-redux

Conversation

@joshistoast
Copy link
Copy Markdown
Collaborator

@joshistoast joshistoast commented May 26, 2026

Summary

Updates the UI for the external providers API keys section in the model manager to be more consistent with the rest of the UI, and addresses some bugs I found.

  • Enhance UI design
  • Add icons for providers
  • Display toasts for updates
  • Address some form submission error handling bugs
  • Address some api key state bugs
image

Related Issues / Discussions

QA Instructions

Merge Plan

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • ❗Changes to a redux slice have a corresponding migration
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions Bot added the frontend PRs that change frontend files label May 26, 2026
- use toasts for updates
- improve state update handling
- pass linting
@joshistoast joshistoast marked this pull request as ready for review May 26, 2026 21:24
@joshistoast joshistoast changed the title feat(model manager): external providers UI tweaks feat(model manager): external providers UI/UX tweaks May 26, 2026
Copy link
Copy Markdown
Collaborator

@Pfannkuchensack Pfannkuchensack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what I see.

@Pfannkuchensack
Copy link
Copy Markdown
Collaborator

Findings

  • High: invokeai/app/api/routers/app_info.py:214 ignores explicit base_url: null payloads, breaking the new "Override Base URL" toggle. The updated frontend in invokeai/frontend/web/src/features/modelManagerV2/subpanels/AddModelPanel/ExternalProviders/ExternalProvidersForm.tsx:192-196 posts { base_url: null } when the user disables the override switch on a provider that already has a custom base URL stored. The route handler guards every field with if update.base_url is not None: (line 214) and therefore drops the null value entirely, so:

    1. The persisted external_<provider>_base_url in api_keys.yaml is never removed - the override silently survives.
    2. When the user disables the toggle without also entering a new API key, the resulting updates dict is empty and the handler raises HTTPException(400, "No external provider config fields provided") at line 218-219. The frontend .catch then shows the generic externalProviderSaveFailed toast added at ExternalProvidersForm.tsx:208-213, even though the user input was valid.

    Pydantic accepts base_url: str | None (line 92) and the generated TS type at invokeai/frontend/web/src/services/api/types.ts:57-60 already exposes string | null, so the wire format is the only thing that's right; the route must distinguish "field omitted" from "field explicitly null". Fix candidate: use update.model_fields_set (or check "base_url" in update.model_fields_set) to detect explicit nulls, mirroring the pattern used in UpdateAppGenerationSettingsRequest.validate_explicit_nulls at lines 135-139.

    To expose this issue, add a test that first POSTs {"api_key": "k", "base_url": "https://x"} to /api/v1/app/external_providers/config/openai, then POSTs {"base_url": null} and asserts the response is 200, payload["base_url"] is None, and load_external_api_keys(...) no longer contains external_openai_base_url while still containing external_openai_api_key.

  • Low: invokeai/frontend/web/public/locales/en.json:3484 adds the key whatsNew.readDocumentation but no caller references it. The "Read the Docs" link in invokeai/frontend/web/src/features/ui/components/WhatsNew.tsx:37 uses whatsNew.readTheDocs instead. The unused key is dead translation surface and will mislead translators. Remove it or wire it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend PRs that change frontend files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants